Skip to content

Commit

Permalink
refactor: modify default from address
Browse files Browse the repository at this point in the history
  • Loading branch information
KishenKumarrrrr committed Aug 8, 2024
1 parent 6fff8b5 commit 5fe2ad2
Show file tree
Hide file tree
Showing 10 changed files with 54 additions and 43 deletions.
4 changes: 1 addition & 3 deletions backend/src/core/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -810,9 +810,7 @@ const config: Config<ConfigSchema> = convict({
})

// If mailFrom was not set in an env var, set it using the app_name
const defaultMailFrom = `${config.get(
'APP_NAME'
)} <[email protected]>`
const defaultMailFrom = `${config.get('APP_NAME')} <[email protected]>`
config.set('mailFrom', config.get('mailFrom') || defaultMailFrom)

// Override some defaults
Expand Down
8 changes: 7 additions & 1 deletion backend/src/core/utils/from-address.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,13 @@ export const isDefaultFromAddress = (from: string): boolean => {
const { fromAddress: defaultFromAddress } = parseFromAddress(
config.get('mailFrom')
)
return fromAddress === defaultFromAddress
// As part of a PSD directive, we have changed the defaultFromAddress to [email protected].
// To prevent any breaking changes, we must now support both the new and old default address
const allowedDefaultAddresses = [
defaultFromAddress,
'[email protected]',
]
return allowedDefaultAddresses.includes(fromAddress)
}

export const fromAddressValidator = Joi.string()
Expand Down
15 changes: 11 additions & 4 deletions backend/src/email/middlewares/email.middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export interface EmailMiddleware {
}

export const INVALID_FROM_ADDRESS_ERROR_MESSAGE =
"Invalid 'from' email address, which must be either the default donotreply@mail.postman.gov.sg or the user's email (which requires setup with Postman team). Contact us to learn more."
"Invalid 'from' email address, which must be either the default info@mail.postman.gov.sg or the user's email (which requires setup with Postman team). Contact us to learn more."

export const UNVERIFIED_FROM_ADDRESS_ERROR_MESSAGE =
'From Address has not been verified. Contact us to learn more.'
Expand Down Expand Up @@ -223,10 +223,17 @@ export const InitEmailMiddleware = (
const { fromName: defaultFromName, fromAddress: defaultFromAddress } =
parseFromAddress(config.get('mailFrom'))

// As part of a PSD directive, we have changed the defaultFromAddress to [email protected].
// To prevent any breaking changes, we must now support both the new and old default address
const allowedDefaultAddresses = [
defaultFromAddress,
'[email protected]',
]

if (
// user enters an email that is neither their own nor donotreply@mail.postman.gov.sg
// user enters an email that is neither their own nor info@mail.postman.gov.sg
fromAddress !== userEmail &&
fromAddress !== defaultFromAddress
allowedDefaultAddresses.includes(fromAddress)
) {
logger.error({
message: INVALID_FROM_ADDRESS_ERROR_MESSAGE,
Expand Down Expand Up @@ -304,7 +311,7 @@ export const InitEmailMiddleware = (

/**
* Verifies the user's email address to see if it can be used as custom 'from' address
* - if it is the default donotreply@mail.postman.gov.sg, return immediately
* - if it is the default info@mail.postman.gov.sg, return immediately
* - else, make network calls to AWS SES and the user's domain to verify DNS settings are set up properly.
*/
const verifyFromAddress = async (
Expand Down
24 changes: 12 additions & 12 deletions backend/src/email/routes/tests/email-campaign.routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe('PUT /campaign/{campaignId}/email/template', () => {
expect.objectContaining({
message: `Template for campaign ${campaignId} updated`,
template: expect.objectContaining({
from: 'Postman <donotreply@mail.postman.gov.sg>',
from: 'Postman <info@mail.postman.gov.sg>',
reply_to: '[email protected]',
}),
})
Expand All @@ -106,7 +106,7 @@ describe('PUT /campaign/{campaignId}/email/template', () => {
const res = await request(app)
.put(`/campaign/${campaignId}/email/template`)
.send({
from: 'Agency.gov.sg <donotreply@mail.postman.gov.sg>',
from: 'Agency.gov.sg <info@mail.postman.gov.sg>',
subject: 'test',
body: 'test',
reply_to: '[email protected]',
Expand All @@ -116,7 +116,7 @@ describe('PUT /campaign/{campaignId}/email/template', () => {
expect.objectContaining({
message: `Template for campaign ${campaignId} updated`,
template: expect.objectContaining({
from: 'Agency.gov.sg via Postman <donotreply@mail.postman.gov.sg>',
from: 'Agency.gov.sg via Postman <info@mail.postman.gov.sg>',
reply_to: '[email protected]',
}),
})
Expand All @@ -127,7 +127,7 @@ describe('PUT /campaign/{campaignId}/email/template', () => {
const res = await request(app)
.put(`/campaign/${campaignId}/email/template`)
.send({
from: 'Postman <donotreply@mail.postman.gov.sg>',
from: 'Postman <info@mail.postman.gov.sg>',
subject: 'test',
body: 'test',
reply_to: '[email protected]',
Expand All @@ -137,7 +137,7 @@ describe('PUT /campaign/{campaignId}/email/template', () => {
expect.objectContaining({
message: `Template for campaign ${campaignId} updated`,
template: expect.objectContaining({
from: 'Postman <donotreply@mail.postman.gov.sg>',
from: 'Postman <info@mail.postman.gov.sg>',
reply_to: '[email protected]',
}),
})
Expand Down Expand Up @@ -194,7 +194,7 @@ describe('PUT /campaign/{campaignId}/email/template', () => {
const res = await request(app)
.put(`/campaign/${campaignId}/email/template`)
.send({
from: 'Custom Name <donotreply@mail.postman.gov.sg>',
from: 'Custom Name <info@mail.postman.gov.sg>',
subject: 'test',
body: 'test',
reply_to: '[email protected]',
Expand All @@ -205,7 +205,7 @@ describe('PUT /campaign/{campaignId}/email/template', () => {
expect.objectContaining({
message: `Template for campaign ${campaignId} updated`,
template: expect.objectContaining({
from: `Custom Name ${mailVia} <donotreply@mail.postman.gov.sg>`,
from: `Custom Name ${mailVia} <info@mail.postman.gov.sg>`,
reply_to: '[email protected]',
}),
})
Expand Down Expand Up @@ -266,7 +266,7 @@ describe('PUT /campaign/{campaignId}/email/template', () => {
const res = await request(app)
.put(`/campaign/${campaignId}/email/template`)
.send({
from: `Custom Name ${mailVia} <donotreply@mail.postman.gov.sg>`,
from: `Custom Name ${mailVia} <info@mail.postman.gov.sg>`,
subject: 'test',
body: 'test',
reply_to: '[email protected]',
Expand All @@ -277,7 +277,7 @@ describe('PUT /campaign/{campaignId}/email/template', () => {
expect.objectContaining({
message: `Template for campaign ${campaignId} updated`,
template: expect.objectContaining({
from: `Custom Name ${mailVia} <donotreply@mail.postman.gov.sg>`,
from: `Custom Name ${mailVia} <info@mail.postman.gov.sg>`,
reply_to: '[email protected]',
}),
})
Expand Down Expand Up @@ -347,7 +347,7 @@ describe('PUT /campaign/{campaignId}/email/template', () => {
expect.objectContaining({
message: `Template for campaign ${protectedCampaignId} updated`,
template: expect.objectContaining({
from: 'Postman <donotreply@mail.postman.gov.sg>',
from: 'Postman <info@mail.postman.gov.sg>',
reply_to: '[email protected]',
}),
})
Expand Down Expand Up @@ -391,7 +391,7 @@ describe('PUT /campaign/{campaignId}/email/template', () => {
message:
'Please re-upload your recipient list as template has changed.',
template: expect.objectContaining({
from: 'Postman <donotreply@mail.postman.gov.sg>',
from: 'Postman <info@mail.postman.gov.sg>',
reply_to: '[email protected]',
}),
})
Expand All @@ -418,7 +418,7 @@ describe('PUT /campaign/{campaignId}/email/template', () => {
template: {
subject: 'test',
body: 'test {{name}}',
from: 'Postman <donotreply@mail.postman.gov.sg>',
from: 'Postman <info@mail.postman.gov.sg>',
reply_to: '[email protected]',
params: ['name'],
},
Expand Down
30 changes: 15 additions & 15 deletions backend/src/email/routes/tests/email-transactional.routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ describe(`${emailTransactionalRoute}/send`, () => {
recipient: '[email protected]',
subject: 'subject',
body: '<p>body</p>',
from: 'Postman <donotreply@mail.postman.gov.sg>',
from: 'Postman <info@mail.postman.gov.sg>',
reply_to: '[email protected]',
}
const generateRandomSmallFile = () => {
Expand Down Expand Up @@ -150,7 +150,7 @@ describe(`${emailTransactionalRoute}/send`, () => {
.spyOn(EmailService, 'sendEmail')
.mockResolvedValue(true)

const from = 'Hello <donotreply@mail.postman.gov.sg>'
const from = 'Hello <info@mail.postman.gov.sg>'
const res = await request(app)
.post(endpoint)
.set('Authorization', `Bearer ${apiKey}`)
Expand All @@ -172,14 +172,14 @@ describe(`${emailTransactionalRoute}/send`, () => {
expect(transactionalEmail).not.toBeNull()
expect(transactionalEmail).toMatchObject({
recipient: validApiCall.recipient,
from: 'Hello <donotreply@mail.postman.gov.sg>',
from: 'Hello <info@mail.postman.gov.sg>',
status: TransactionalEmailMessageStatus.Accepted,
errorCode: null,
})
expect(transactionalEmail?.params).toMatchObject({
subject: validApiCall.subject,
body: validApiCall.body,
from: 'Hello <donotreply@mail.postman.gov.sg>',
from: 'Hello <info@mail.postman.gov.sg>',
reply_to: user.email,
})
})
Expand Down Expand Up @@ -479,7 +479,7 @@ describe(`${emailTransactionalRoute}/send`, () => {
.field('recipient', validApiCallAttachment.recipient)
.field('subject', validApiCallAttachment.subject)
.field('body', validApiCallAttachment.body)
.field('from', 'Postman <donotreply@mail.postman.gov.sg>')
.field('from', 'Postman <info@mail.postman.gov.sg>')
.field('reply_to', validApiCallAttachment.reply_to)
.attach('attachments', validAttachment, validAttachmentName)
expect(res.status).toBe(403)
Expand Down Expand Up @@ -971,29 +971,29 @@ describe(`GET ${emailTransactionalRoute}`, () => {
const endpoint = emailTransactionalRoute
const acceptedMessage = {
recipient: '[email protected]',
from: 'Postman <donotreply@mail.postman.gov.sg>',
from: 'Postman <info@mail.postman.gov.sg>',
params: {
from: 'Postman <donotreply@mail.postman.gov.sg>',
from: 'Postman <info@mail.postman.gov.sg>',
subject: 'Test',
body: 'Test Body',
},
status: TransactionalEmailMessageStatus.Accepted,
}
const sentMessage = {
recipient: '[email protected]',
from: 'Postman <donotreply@mail.postman.gov.sg>',
from: 'Postman <info@mail.postman.gov.sg>',
params: {
from: 'Postman <donotreply@mail.postman.gov.sg>',
from: 'Postman <info@mail.postman.gov.sg>',
subject: 'Test',
body: 'Test Body',
},
status: TransactionalEmailMessageStatus.Sent,
}
const deliveredMessage = {
recipient: '[email protected]',
from: 'Postman <donotreply@mail.postman.gov.sg>',
from: 'Postman <info@mail.postman.gov.sg>',
params: {
from: 'Postman <donotreply@mail.postman.gov.sg>',
from: 'Postman <info@mail.postman.gov.sg>',
subject: 'Test',
body: 'Test Body',
},
Expand Down Expand Up @@ -1330,9 +1330,9 @@ describe(`GET ${emailTransactionalRoute}/:emailId`, () => {
const message = await EmailMessageTransactional.create({
userId: user.id,
recipient: '[email protected]',
from: 'Postman <donotreply@mail.postman.gov.sg>',
from: 'Postman <info@mail.postman.gov.sg>',
params: {
from: 'Postman <donotreply@mail.postman.gov.sg>',
from: 'Postman <info@mail.postman.gov.sg>',
subject: 'Test',
body: 'Test Body',
},
Expand Down Expand Up @@ -1368,9 +1368,9 @@ describe(`GET ${emailTransactionalRoute}/:emailId`, () => {
const message = await EmailMessageTransactional.create({
userId: user.id,
recipient: '[email protected]',
from: 'Postman <donotreply@mail.postman.gov.sg>',
from: 'Postman <info@mail.postman.gov.sg>',
params: {
from: 'Postman <donotreply@mail.postman.gov.sg>',
from: 'Postman <info@mail.postman.gov.sg>',
subject: 'Test',
body: 'Test Body',
},
Expand Down
2 changes: 1 addition & 1 deletion backend/src/test-utils/test-env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ process.env.SENDGRID_PUBLIC_KEY =
process.env.SESSION_SECRET = 'SESSIONSECRET'
process.env.DB_URI =
'postgres://postgres:postgres@localhost:5432/postmangovsg_test'
process.env.BACKEND_SES_FROM = 'Postman <donotreply@mail.postman.gov.sg>'
process.env.BACKEND_SES_FROM = 'Postman <info@mail.postman.gov.sg>'
process.env.API_KEY_SALT_V1 = bcrypt.genSaltSync(1)
process.env.TRANSACTIONAL_EMAIL_RATE = '1'
process.env.TWILIO_CREDENTIAL_CACHE_MAX_AGE = '0'
Expand Down
8 changes: 4 additions & 4 deletions e2e/config.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
export const API_URL =
process.env.API_URL || 'https://api-staging.postman.gov.sg';
process.env.API_URL || "https://api-staging.postman.gov.sg";
export const API_KEY = process.env.API_KEY as string;
export const MAILBOX = process.env.MAIL_BOX || '[email protected]';
export const MAILBOX = process.env.MAIL_BOX || "[email protected]";

export const DASHBOARD_URL =
process.env.DASHBOARD_URL || 'https://staging.postman.gov.sg';
export const POSTMAN_FROM = 'donotreply@mail.postman.gov.sg';
process.env.DASHBOARD_URL || "https://staging.postman.gov.sg";
export const POSTMAN_FROM = "info@mail.postman.gov.sg";

export const SMS_NUMBER = process.env.SMS_NUMBER as string;
export const TWILIO_ACC_SID = process.env.TWILIO_ACC_SID as string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ const EmailTemplate = ({
parseFromAddress(selectedFrom)

// Use custom from name if it has already been set. For e.g.,
// for "Custom <donotreply@mail.postman.gov.sg>"", we should
// for "Custom <info@mail.postman.gov.sg>"", we should
// use "Custom" instead of the default "Postman.gov.sg".
setFromName(
selectedFromAddress === initialFromAddress
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/locales/en/messages.po
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ msgstr "Want to send your campaigns through WhatsApp?"
#: components/dashboard/settings/custom-from-address/CustomFromAddress.tsx:112
#: config.ts:87
msgid "defaultMailFrom"
msgstr "donotreply@mail.postman.gov.sg"
msgstr "info@mail.postman.gov.sg"

#: components/dashboard/demo/demo-info-banner/DemoInfoBanner.tsx:8
msgid "demoMessage"
Expand Down
2 changes: 1 addition & 1 deletion worker/src/core/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ const config: Config<ConfigSchema> = convict({
})

// If mailFrom was not set in an env var, set it using the app_name
const defaultMailFrom = 'Postman.gov.sg <donotreply@mail.postman.gov.sg>'
const defaultMailFrom = 'Postman.gov.sg <info@mail.postman.gov.sg>'
config.set('mailFrom', config.get('mailFrom') || defaultMailFrom)

// Only development is a non-production environment
Expand Down

0 comments on commit 5fe2ad2

Please sign in to comment.