Skip to content

Commit

Permalink
UBERF-9577: Fix using default from address in emails (#8163)
Browse files Browse the repository at this point in the history
Signed-off-by: Artem Savchenko <[email protected]>
  • Loading branch information
ArtyomSavchenko authored Mar 7, 2025
1 parent 14a6a3a commit 427ef59
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 8 deletions.
1 change: 1 addition & 0 deletions services/mail/pod-mail/src/__tests__/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ describe('Config', () => {
process.env.SMTP_HOST = 'smtp.example.com'
process.env.SMTP_PORT = '587'
process.env.SMTP_USERNAME = 'user'
process.env.SMTP_PASSWORD = undefined

// eslint-disable-next-line @typescript-eslint/no-var-requires
const loadedConfig = require('../config').default
Expand Down
49 changes: 47 additions & 2 deletions services/mail/pod-mail/src/__tests__/main.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,15 @@ jest.mock('../mail', () => ({
sendMessage: jest.fn()
}))
}))
jest.mock('../config', () => ({}))
jest.mock('../config', () => ({
source: '[email protected]'
}))

describe('handleSendMail', () => {
let req: Request
let res: Response
let sendMailMock: jest.Mock
let mailClient: MailClient

beforeEach(() => {
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions
Expand All @@ -44,7 +47,8 @@ describe('handleSendMail', () => {
send: jest.fn()
} as unknown as Response

sendMailMock = (new MailClient().sendMessage as jest.Mock).mockResolvedValue({})
mailClient = new MailClient()
sendMailMock = (mailClient.sendMessage as jest.Mock).mockResolvedValue({})
})

it('should return 400 if text is missing', async () => {
Expand Down Expand Up @@ -84,4 +88,45 @@ describe('handleSendMail', () => {

expect(res.send).toHaveBeenCalled() // Check that a response is still sent
})

it('should use source from config if from is not provided', async () => {
await handleSendMail(mailClient, req, res)

expect(sendMailMock).toHaveBeenCalledWith(
expect.objectContaining({
from: '[email protected]', // Verify that the default source from config is used
to: '[email protected]',
subject: 'Test Subject',
text: 'Hello, world!'
})
)
})

it('should use from if it is provided', async () => {
req.body.from = '[email protected]'
await handleSendMail(mailClient, req, res)

expect(sendMailMock).toHaveBeenCalledWith(
expect.objectContaining({
from: '[email protected]', // Verify that the from is used
to: '[email protected]',
subject: 'Test Subject',
text: 'Hello, world!'
})
)
})

it('should send to multiple addresses', async () => {
req.body.to = ['[email protected]', '[email protected]']
await handleSendMail(mailClient, req, res)

expect(sendMailMock).toHaveBeenCalledWith(
expect.objectContaining({
from: '[email protected]',
to: ['[email protected]', '[email protected]'], // Verify that multiple addresses are passed
subject: 'Test Subject',
text: 'Hello, world!'
})
)
})
})
6 changes: 4 additions & 2 deletions services/mail/pod-mail/src/mail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@ export class MailClient {

async sendMessage (message: SendMailOptions): Promise<void> {
this.transporter.sendMail(message, (err, info) => {
const messageInfo = `(from: ${message.from as string}, to: ${message.to as string})`
if (err !== null) {
console.error('Failed to send email: ', err.message)
console.error(`Failed to send email ${messageInfo}: `, err.message)
console.log('Failed message details: ', message)
} else {
console.log(`Email request ${info?.messageId} sent: ${info?.response}`)
console.log(`Email request ${messageInfo} sent: ${info?.response}`)
}
})
}
Expand Down
50 changes: 46 additions & 4 deletions services/mail/pod-mail/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import { type SendMailOptions } from 'nodemailer'
import { Request, Response } from 'express'
import Mail from 'nodemailer/lib/mailer'

import config from './config'
import { createServer, listen } from './server'
Expand Down Expand Up @@ -55,19 +56,36 @@ export const main = async (): Promise<void> => {

export async function handleSendMail (client: MailClient, req: Request, res: Response): Promise<void> {
// Skip auth check, since service should be internal
const message: SendMailOptions = req.body
if (message?.text === undefined) {
const { from, to, subject, text, html, attachments } = req.body
const fromAddress = from ?? config.source
if (text === undefined) {
res.status(400).send({ err: "'text' is missing" })
return
}
if (message?.subject === undefined) {
if (subject === undefined) {
res.status(400).send({ err: "'subject' is missing" })
return
}
if (message?.to === undefined) {
if (to === undefined) {
res.status(400).send({ err: "'to' is missing" })
return
}
if (fromAddress === undefined) {
res.status(400).send({ err: "'from' is missing" })
return
}
const message: SendMailOptions = {
from: fromAddress,
to,
subject,
text
}
if (html !== undefined) {
message.html = html
}
if (attachments !== undefined) {
message.attachments = getAttachments(attachments)
}
try {
await client.sendMessage(message)
} catch (err) {
Expand All @@ -76,3 +94,27 @@ export async function handleSendMail (client: MailClient, req: Request, res: Res

res.send()
}

function getAttachments (attachments: any): Mail.Attachment[] | undefined {
if (attachments === undefined || attachments === null) {
return undefined
}
if (!Array.isArray(attachments)) {
console.error('attachments is not array')
return undefined
}
return attachments.map((a) => {
const attachment: Mail.Attachment = {
content: a.content,
contentType: a.contentType,
path: a.path,
filename: a.filename,
cid: a.cid,
encoding: a.encoding,
contentTransferEncoding: a.contentTransferEncoding,
headers: a.headers,
raw: a.raw
}
return attachment
})
}

0 comments on commit 427ef59

Please sign in to comment.