fix: replace credential hash with uuid

opengovsg · Sep 5, 2023 · d571b33 · d571b33
1 parent f0eb774
commit d571b33
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 35 deletions.
diff --git a/backend/src/sms/middlewares/sms.middleware.ts b/backend/src/sms/middlewares/sms.middleware.ts
@@ -10,6 +10,7 @@ import {
   ApiInvalidCredentialsError,
   ApiNotFoundError,
 } from '@core/errors/rest-api.errors'
+import { v4 as uuid } from 'uuid'
 
 export interface SmsMiddleware {
   getCredentialsFromBody: Handler
@@ -241,9 +242,7 @@ export const InitSmsMiddleware = (
     try {
       // Store credentials in AWS secrets manager
       const stringifiedCredential = JSON.stringify(credentials)
-      const credentialName = await SmsService.getEncodedHash(
-        stringifiedCredential
-      )
+      const credentialName = uuid()
 
       // We only want to tag Twilio credentials with the environment tag in SecretsManager
       // when env is development. This allows us to quickly identify and clean up dev secrets.

diff --git a/backend/src/sms/routes/tests/sms-campaign.routes.test.ts b/backend/src/sms/routes/tests/sms-campaign.routes.test.ts
@@ -226,11 +226,7 @@ describe('POST /campaign/{campaignId}/sms/new-credentials', () => {
       .spyOn(SmsService, 'sendCampaignMessage')
       .mockResolvedValue()
 
-    // getEncodedHash is used as the stored name in AWS SecretsManager
-    const HASHED_CREDS = 'HASHED_CREDS'
-    const mockGetEncodedHash = jest
-      .spyOn(SmsService, 'getEncodedHash')
-      .mockResolvedValue(HASHED_CREDS)
+    const EXPECTED_CRED_NAME = 'HASHED_CREDS'
 
     const res = await request(app)
       .post(`/campaign/${nonDemoCampaign.id}/sms/new-credentials`)
@@ -246,7 +242,7 @@ describe('POST /campaign/{campaignId}/sms/new-credentials', () => {
 
     expect(mockSecretsManager.createSecret).toHaveBeenCalledWith(
       expect.objectContaining({
-        Name: HASHED_CREDS,
+        Name: EXPECTED_CRED_NAME,
         SecretString: JSON.stringify({
           accountSid: 'twilio_account_sid',
           apiKey: 'twilio_api_key',
@@ -259,12 +255,11 @@ describe('POST /campaign/{campaignId}/sms/new-credentials', () => {
     // Ensure credential was added into DB
     const dbCredential = await Credential.findOne({
       where: {
-        name: HASHED_CREDS,
+        name: EXPECTED_CRED_NAME,
       },
     })
     expect(dbCredential).not.toBe(null)
     mockSendCampaignMessage.mockRestore()
-    mockGetEncodedHash.mockRestore()
   })
 })
 

diff --git a/backend/src/sms/routes/tests/sms-settings.routes.test.ts b/backend/src/sms/routes/tests/sms-settings.routes.test.ts
@@ -59,12 +59,7 @@ describe('POST /settings/sms/credentials', () => {
     const mockSendValidationMessage = jest
       .spyOn(SmsService, 'sendValidationMessage')
       .mockResolvedValue()
-
-    // getEncodedHash is used as the stored name in AWS SecretsManager
-    const HASHED_CREDS = 'HASHED_CREDS'
-    const mockGetEncodedHash = jest
-      .spyOn(SmsService, 'getEncodedHash')
-      .mockResolvedValue(HASHED_CREDS)
+    const EXPECTED_CRED_NAME = 'HASHED_CREDS'
 
     const res = await request(app).post('/settings/sms/credentials').send({
       label: CREDENTIAL_LABEL,
@@ -79,7 +74,7 @@ describe('POST /settings/sms/credentials', () => {
 
     expect(mockSecretsManager.createSecret).toHaveBeenCalledWith(
       expect.objectContaining({
-        Name: HASHED_CREDS,
+        Name: EXPECTED_CRED_NAME,
         SecretString: JSON.stringify({
           accountSid: 'twilio_account_sid',
           apiKey: 'twilio_api_key',
@@ -92,7 +87,7 @@ describe('POST /settings/sms/credentials', () => {
     // Ensure credential was added into DB
     const dbCredential = await Credential.findOne({
       where: {
-        name: HASHED_CREDS,
+        name: EXPECTED_CRED_NAME,
       },
     })
     expect(dbCredential).not.toBe(null)
@@ -101,12 +96,11 @@ describe('POST /settings/sms/credentials', () => {
       where: {
         label: CREDENTIAL_LABEL,
         type: ChannelType.SMS,
-        credName: HASHED_CREDS,
+        credName: EXPECTED_CRED_NAME,
         userId: 1,
       },
     })
     expect(dbUserCredential).not.toBe(null)
     mockSendValidationMessage.mockRestore()
-    mockGetEncodedHash.mockRestore()
   })
 })
diff --git a/backend/src/sms/services/sms.service.ts b/backend/src/sms/services/sms.service.ts
@@ -1,4 +1,3 @@
-import bcrypt from 'bcrypt'
 import { Transaction } from 'sequelize'
 
 import config from '@core/config'
@@ -181,18 +180,6 @@ const getCampaignDetails = async (
   return { ...campaignDetails, cost_per_message: costPerSms }
 }
 
-/**
- * Returns a base 64 encoded hash for secrets manager
- * @param secret
- */
-const getEncodedHash = async (secret: string): Promise<string> => {
-  const secretHash = await bcrypt.hash(
-    secret,
-    config.get('aws.secretManagerSalt')
-  )
-  return Buffer.from(secretHash).toString('base64')
-}
-
 const uploadCompleteOnPreview = ({
   transaction,
   template,
@@ -309,7 +296,6 @@ const getTwilioCostPerOutgoingSMSSegmentUSD = async (
 }
 
 export const SmsService = {
-  getEncodedHash,
   findCampaign,
   getCampaignDetails,
   getHydratedMessage,