-
Notifications
You must be signed in to change notification settings - Fork 1
Domains, messages, attachments, tests #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
de336a8
a7b2890
f6f05a9
723c50b
4c62361
797d6bd
a44ea0d
a5a146e
fbe6bee
51944ac
ec56349
1ee1cf3
74391bd
2744c92
199ed6c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| import { resolveMx } from 'node:dns/promises'; | ||
| import type { MxRecord } from 'node:dns'; | ||
|
|
||
| export async function getDNSMXRecords({ | ||
| domain, | ||
| }: { | ||
| domain: string; | ||
| }): Promise<MxRecord[]> { | ||
| try { | ||
| return await resolveMx(domain); | ||
| } catch (error) { | ||
| console.error('Error resolving MX records:', error); | ||
| return []; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| import mongoose, { type HydratedDocument } from "mongoose"; | ||
| import Domain from "../db/mongo/schemas/Domain"; | ||
| import { getDNSMXRecords } from "./DNSController"; | ||
| import type IDomain from "../models/Domain"; | ||
|
|
||
| export async function createDomain({ | ||
| organizationId, | ||
| name, | ||
| }: { | ||
| organizationId: string; | ||
| name: string; | ||
| }) { | ||
| const domain = new Domain(); | ||
| domain.organizationId = new mongoose.Types.ObjectId(organizationId); | ||
| domain.name = name; | ||
| await domain.save(); | ||
| return domain; | ||
| } | ||
|
Comment on lines
+6
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainAdd input validation and error handling. The function lacks:
Consider adding validation and try-catch blocks for database operations. Run the following script to check if validation is handled at the route layer: 🏁 Script executed: #!/bin/bash
# Check if domain routes include validation middleware or error handling
fd -e ts -e js "domains" api/routes | xargs rg -A 10 "createDomain|validation|validate"Length of output: 3631 Length of output: 1227 Length of output: 266 Add error handling for database operations. Input validation is already handled at the route layer via express-validator (
Wrap the 🤖 Prompt for AI Agents |
||
|
|
||
| export async function getDomainsByOrganizationId({ | ||
| organizationId, | ||
| }: { | ||
| organizationId: string; | ||
| }) { | ||
| return await Domain.find({ | ||
| organizationId: new mongoose.Types.ObjectId(organizationId), | ||
| }); | ||
| } | ||
|
|
||
| export async function getDomainByOrganizationIdAndName({ | ||
| organizationId, | ||
| name, | ||
| }: { | ||
| organizationId: string; | ||
| name: string; | ||
| }) { | ||
| return await Domain.findOne({ | ||
| organizationId: new mongoose.Types.ObjectId(organizationId), | ||
| name, | ||
| }); | ||
| } | ||
|
|
||
| export async function getVerifiedDomainByOrganizationIdAndName({ | ||
| organizationId, | ||
| name, | ||
| }: { | ||
| organizationId: string; | ||
| name: string; | ||
| }) { | ||
| return await Domain.findOne({ | ||
| organizationId: new mongoose.Types.ObjectId(organizationId), | ||
| name: name, | ||
| verified: true, | ||
| }); | ||
| } | ||
|
|
||
| export async function deleteDomainByOrganizationIdAndName({ | ||
| organizationId, | ||
| name, | ||
| }: { | ||
| organizationId: string; | ||
| name: string; | ||
| }) { | ||
| return await Domain.findOneAndDelete({ | ||
| organizationId: new mongoose.Types.ObjectId(organizationId), | ||
| name: name, | ||
| }); | ||
| } | ||
|
|
||
| export async function verifyDomainDNS({ | ||
| domain, | ||
| }: { | ||
| domain: HydratedDocument<IDomain>; | ||
| }): Promise<{ verified: boolean; domain: HydratedDocument<IDomain> }> { | ||
| const mxRecords = await getDNSMXRecords({ domain: domain.name }); | ||
|
|
||
| let mxRecordFound; | ||
| if (mxRecords && Array.isArray(mxRecords)) { | ||
| mxRecordFound = domain.records.find( | ||
| (domainRecord) => | ||
| domainRecord.type === "MX" && | ||
| mxRecords.find((dnsRecord) => { | ||
| return ( | ||
| typeof dnsRecord.exchange === "string" && | ||
| dnsRecord.exchange.toLowerCase() === domainRecord.value.toLowerCase() | ||
| ); | ||
| }) | ||
| ); | ||
| } | ||
|
|
||
| if (mxRecordFound) { | ||
| mxRecordFound.status = "verified"; | ||
| } | ||
| domain.verified = mxRecordFound ? true : false; | ||
| await domain.save(); | ||
|
|
||
| return { verified: domain.verified, domain }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,10 @@ | ||
| import { | ||
| GetIdentityDkimAttributesCommand, | ||
| SendEmailCommand, | ||
| SendRawEmailCommand, | ||
| SESClient, | ||
| VerifyDomainDkimCommand, | ||
| } from "@aws-sdk/client-ses"; | ||
| import MailComposer from "nodemailer/lib/mail-composer"; | ||
| import type { SNSMessage } from "../models/SES"; | ||
| import { createMessage, getMessageByExternalMessageId, getMessageById } from "./MessageController"; | ||
| import type { MessageStatus } from "../models/Message"; | ||
|
|
@@ -38,46 +39,68 @@ export async function sendSESMessage({ | |
| from, | ||
| fromName, | ||
| to, | ||
| cc, | ||
| bcc, | ||
| subject, | ||
| text, | ||
| html, | ||
| attachments, | ||
| }: { | ||
| messageId: string; | ||
| from: string; | ||
| fromName?: string; | ||
| to: string; | ||
| to?: string[]; | ||
| cc?: string[]; | ||
| bcc?: string[]; | ||
| subject: string; | ||
| text: string; | ||
| html: string; | ||
| attachments?: { | ||
| content: string; | ||
| name?: string; | ||
| contentType?: string; | ||
| }[]; | ||
| }) { | ||
| const command = new SendEmailCommand({ | ||
| Source: fromName ? `${fromName} <${from}>` : from, | ||
| Destination: { | ||
| ToAddresses: [to], | ||
| }, | ||
| Message: { | ||
| Subject: { | ||
| Data: subject, | ||
| Charset: "UTF-8", | ||
| }, | ||
| Body: { | ||
| Html: { | ||
| Data: html, | ||
| Charset: "UTF-8", | ||
| }, | ||
| Text: { | ||
| Data: text, | ||
| Charset: "UTF-8", | ||
| }, | ||
| }, | ||
| // SES SendEmailCommand does NOT support attachments. To include attachments, you must use SendRawEmailCommand and manually build a MIME-encoded message. | ||
| // Here's one way to do it with attachments: | ||
|
|
||
| // Create the MIME message using nodemailer's MailComposer | ||
| const mailOptions: any = { | ||
| from: fromName ? `"${fromName}" <${from}>` : from, | ||
| to: (to ?? []).join(", "), | ||
| cc: (cc ?? []).join(", "), | ||
| bcc: (bcc ?? []).join(", "), | ||
| subject, | ||
| text, | ||
| html, | ||
| attachments: (attachments ?? []).map(att => ({ | ||
| filename: att.name, | ||
| content: att.content, | ||
| contentType: att.contentType, | ||
| encoding: "base64", | ||
| })), | ||
|
Comment on lines
+76
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Add attachment size validation. The code maps attachments without validating their size. Since attachments are base64-encoded and stored in MongoDB (16MB document limit), large attachments could cause failures. Add validation to prevent oversized attachments. Add validation before the MailComposer: + // Validate total attachment size (MongoDB 16MB limit, leave buffer for other fields)
+ const MAX_ATTACHMENT_SIZE = 10 * 1024 * 1024; // 10MB
+ const totalAttachmentSize = (attachments ?? []).reduce((sum, att) => {
+ // Base64 encoded size is ~4/3 of original
+ const decodedSize = (att.content.length * 3) / 4;
+ return sum + decodedSize;
+ }, 0);
+
+ if (totalAttachmentSize > MAX_ATTACHMENT_SIZE) {
+ throw new Error(`Total attachment size exceeds ${MAX_ATTACHMENT_SIZE} bytes`);
+ }
const mailOptions: any = {
// ... rest of the code
|
||
| headers: { | ||
| "X-SES-CONFIGURATION-SET": "sendook-config-set", | ||
| "X-SES-MESSAGE-TAGS": `message=${messageId}`, | ||
| }, | ||
| }; | ||
|
|
||
| const composer = new MailComposer(mailOptions); | ||
| const mimeMessage = await new Promise<Buffer>((resolve, reject) => { | ||
| composer.compile().build((err, message) => { | ||
| if (err) return reject(err); | ||
| resolve(message); | ||
| }); | ||
| }); | ||
|
|
||
| const command = new SendRawEmailCommand({ | ||
| RawMessage: { Data: mimeMessage }, | ||
| Tags: [ | ||
| { | ||
| Name: "message", | ||
| Value: messageId, | ||
| }, | ||
| ], | ||
| ConfigurationSetName: "sendook-config-set", | ||
| }); | ||
| return await ses.send(command); | ||
| } | ||
|
|
@@ -150,7 +173,7 @@ export async function handleInboundSESMessage({ | |
| threadId, | ||
| from: notification.mail.source, | ||
| fromInboxId: fromInboxId?.id, | ||
| to: notification.mail.destination[0], | ||
| to: [notification.mail.destination[0]], | ||
| toInboxId: inbox.id, | ||
| subject: notification.mail.commonHeaders?.subject, | ||
| text: content.getVisibleText(), | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error handling may mask DNS lookup failures.
Returning an empty array on error makes it impossible for callers to distinguish between legitimate scenarios (no MX records configured) and transient failures (network issues, DNS timeouts). This could lead to false negatives in domain verification flows, where a temporary DNS failure is interpreted as "domain has no MX records."
Consider either:
{ success: boolean, records: MxRecord[], error?: Error }🤖 Prompt for AI Agents