diff --git a/packages/api/src/routers/svc/emails.ts b/packages/api/src/routers/svc/emails.ts index 01498f83a..0c166cdf6 100644 --- a/packages/api/src/routers/svc/emails.ts +++ b/packages/api/src/routers/svc/emails.ts @@ -1,12 +1,22 @@ import express from 'express' -import { readPushSubscription } from '../../datalayer/pubsub' +import { + createPubSubClient, + readPushSubscription, +} from '../../datalayer/pubsub' import { sendEmail } from '../../utils/sendEmail' import { analytics } from '../../utils/analytics' import { getNewsletterEmail } from '../../services/newsletters' import { env } from '../../env' -import { v4 as uuid } from 'uuid' -import { findNewsletterUrl, isProbablyNewsletter } from '../../utils/parser' +import { + findNewsletterUrl, + generateUniqueUrl, + getTitleFromEmailSubject, + isProbablyArticle, + isProbablyNewsletter, +} from '../../utils/parser' import { saveNewsletterEmail } from '../../services/save_newsletter_email' +import { saveEmail } from '../../services/save_email' +import { buildLogger } from '../../utils/logger' interface ForwardEmailMessage { from: string @@ -17,15 +27,17 @@ interface ForwardEmailMessage { unsubHttpUrl?: string } +const logger = buildLogger('app.dispatch') + export function emailsServiceRouter() { const router = express.Router() // eslint-disable-next-line @typescript-eslint/no-misused-promises router.post('/forward', async (req, res) => { - console.log('forward') + logger.info('email forward router') const { message, expired } = readPushSubscription(req) - console.log('pubsub message:', message, 'expired:', expired) + logger.info('pubsub message:', message, 'expired:', expired) if (!message) { res.status(400).send('Bad Request') @@ -33,7 +45,7 @@ export function emailsServiceRouter() { } if (expired) { - console.log('discards expired message:', message) + logger.log('discards expired message:', message) res.status(200).send('Expired') return } @@ -48,39 +60,55 @@ export function emailsServiceRouter() { !('subject' in data) || !('html' in data) ) { - console.log('Invalid message') + logger.info('Invalid message') res.status(400).send('Bad Request') return } - if (await isProbablyNewsletter(data.html)) { - console.log('handling as newsletter', data) - await saveNewsletterEmail({ - email: data.to, - title: data.subject, - content: data.html, - author: data.from, - url: - (await findNewsletterUrl(data.html)) || - 'https://omnivore.app/no_url?q' + uuid(), - unsubMailTo: data.unsubMailTo, - unsubHttpUrl: data.unsubHttpUrl, - }) - res.status(200).send('Newsletter') - return - } - // get user from newsletter email const newsletterEmail = await getNewsletterEmail(data.to) if (!newsletterEmail) { - console.log('newsletter email not found', data.to) + logger.info('newsletter email not found', data.to) res.status(200).send('Not Found') return } + const user = newsletterEmail.user + const ctx = { pubsub: createPubSubClient(), uid: user.id } + + if (await isProbablyNewsletter(data.html)) { + logger.info('handling as newsletter', data) + await saveNewsletterEmail( + { + email: data.to, + title: data.subject, + content: data.html, + author: data.from, + url: (await findNewsletterUrl(data.html)) || generateUniqueUrl(), + unsubMailTo: data.unsubMailTo, + unsubHttpUrl: data.unsubHttpUrl, + newsletterEmail, + }, + ctx + ) + res.status(200).send('Newsletter') + return + } + + if (await isProbablyArticle(data.from, data.subject)) { + logger.info('handling as article', data) + await saveEmail(ctx, { + title: getTitleFromEmailSubject(data.subject), + author: data.from, + url: generateUniqueUrl(), + originalContent: data.html, + }) + res.status(200).send('Article') + return + } analytics.track({ - userId: newsletterEmail.user.id, + userId: user.id, event: 'non_newsletter_email_received', properties: { env: env.server.apiEnv, @@ -90,21 +118,21 @@ export function emailsServiceRouter() { // forward non-newsletter emails to the registered email address const result = await sendEmail({ from: env.sender.message, - to: newsletterEmail.user.email, + to: user.email, subject: `Fwd: ${data.subject}`, html: data.html, replyTo: data.from, }) if (!result) { - console.log('Email not forwarded', data) + logger.info('Email not forwarded', data) res.status(200).send('Failed to send email') return } res.status(200).send('Email forwarded') } catch (e) { - console.log(e) + logger.info(e) if (e instanceof SyntaxError) { // when message is not a valid json string res.status(400).send(e) diff --git a/packages/api/src/routers/svc/newsletters.ts b/packages/api/src/routers/svc/newsletters.ts index 463da1747..f76536098 100644 --- a/packages/api/src/routers/svc/newsletters.ts +++ b/packages/api/src/routers/svc/newsletters.ts @@ -96,7 +96,7 @@ export function newsletterServiceRouter() { const result = await saveNewsletterEmail(data) if (!result) { - console.log('Error createing newsletter link from data', data) + console.log('Error creating newsletter link from data', data) res.status(500).send('Error creating newsletter link') return } diff --git a/packages/api/src/services/save_newsletter_email.ts b/packages/api/src/services/save_newsletter_email.ts index ec06d6a9e..ecf5177dd 100644 --- a/packages/api/src/services/save_newsletter_email.ts +++ b/packages/api/src/services/save_newsletter_email.ts @@ -11,6 +11,7 @@ import { getDeviceTokensByUserId } from './user_device_tokens' import { Page } from '../elastic/types' import { addLabelToPage } from './labels' import { saveSubscription } from './subscriptions' +import { NewsletterEmail } from '../entity/newsletter_email' interface NewsletterMessage { email: string @@ -20,6 +21,7 @@ interface NewsletterMessage { author: string unsubMailTo?: string unsubHttpUrl?: string + newsletterEmail?: NewsletterEmail } // Returns true if the link was created successfully. Can still fail to @@ -29,7 +31,8 @@ export const saveNewsletterEmail = async ( ctx?: SaveContext ): Promise => { // get user from newsletter email - const newsletterEmail = await getNewsletterEmail(data.email) + const newsletterEmail = + data.newsletterEmail || (await getNewsletterEmail(data.email)) if (!newsletterEmail) { console.log('newsletter email not found', data.email) diff --git a/packages/api/src/utils/parser.ts b/packages/api/src/utils/parser.ts index 7489b7373..89fe4b018 100644 --- a/packages/api/src/utils/parser.ts +++ b/packages/api/src/utils/parser.ts @@ -15,6 +15,10 @@ import { GolangHandler } from './golang-handler' import * as hljs from 'highlightjs' import { decode } from 'html-entities' import { parseHTML } from 'linkedom' +import { getRepository } from '../entity/utils' +import { User } from '../entity/user' +import { ILike } from 'typeorm' +import { v4 as uuid } from 'uuid' const logger = buildLogger('utils.parse') @@ -37,6 +41,7 @@ const DOM_PURIFY_CONFIG = { 'data-feature', ], } +const ARTICLE_PREFIX = 'omnivore:' interface ContentHandler { shouldPrehandle: (url: URL, dom: Document) => boolean @@ -545,3 +550,20 @@ export const findNewsletterUrl = async ( return undefined } + +export const isProbablyArticle = async ( + email: string, + subject: string +): Promise => { + const user = await getRepository(User).findOneBy({ + email: ILike(email), + }) + return !!user || subject.includes(ARTICLE_PREFIX) +} + +export const generateUniqueUrl = () => 'https://omnivore.app/no_url?q=' + uuid() + +export const getTitleFromEmailSubject = (subject: string) => { + const title = subject.replace(ARTICLE_PREFIX, '') + return title.trim() +} diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index 721f19c55..b45fe926c 100644 --- a/packages/api/test/routers/auth.test.ts +++ b/packages/api/test/routers/auth.test.ts @@ -1,6 +1,5 @@ import { createTestUser, deleteTestUser } from '../db' import { generateFakeUuid, request } from '../util' -import { expect } from 'chai' import { StatusType } from '../../src/datalayer/user/model' import { getRepository } from '../../src/entity/utils' import { User } from '../../src/entity/user' @@ -13,6 +12,10 @@ import { generateVerificationToken, hashPassword, } from '../../src/utils/auth' +import sinonChai from 'sinon-chai' +import chai, { expect } from 'chai' + +chai.use(sinonChai) describe('auth router', () => { const route = '/api/auth' diff --git a/packages/api/test/routers/emails.test.ts b/packages/api/test/routers/emails.test.ts new file mode 100644 index 000000000..26220d656 --- /dev/null +++ b/packages/api/test/routers/emails.test.ts @@ -0,0 +1,131 @@ +import { + createTestNewsletterEmail, + createTestUser, + deleteTestUser, +} from '../db' +import { User } from '../../src/entity/user' +import 'mocha' +import sinon from 'sinon' +import { expect } from 'chai' +import { request } from '../util' +import * as parser from '../../src/utils/parser' +import * as sendNotification from '../../src/utils/sendNotification' +import * as sendEmail from '../../src/utils/sendEmail' + +describe('Emails Router', () => { + const username = 'fakeUser' + const newsletterEmail = 'fakeUser@omnivore.app' + + let user: User + let token: string + + before(async () => { + // create test user and login + user = await createTestUser(username) + + await createTestNewsletterEmail(user, newsletterEmail) + token = process.env.PUBSUB_VERIFICATION_TOKEN! + }) + + after(async () => { + // clean up + await deleteTestUser(username) + sinon.restore() + }) + + describe('forward', () => { + const from = 'from@omnivore.app' + const to = newsletterEmail + const subject = 'test subject' + const html = 'test html' + + beforeEach(async () => { + sinon.replace( + sendNotification, + 'sendMulticastPushNotifications', + sinon.fake.resolves(undefined) + ) + sinon.replace(sendEmail, 'sendEmail', sinon.fake.resolves(true)) + }) + + afterEach(() => { + sinon.restore() + }) + + context('when email is a newsletter', () => { + before(() => { + sinon.replace(parser, 'isProbablyNewsletter', sinon.fake.resolves(true)) + }) + + it('saves the email as a newsletter', async () => { + const data = { + message: { + data: Buffer.from( + JSON.stringify({ from, to, subject, html }) + ).toString('base64'), + publishTime: new Date().toISOString(), + }, + } + const res = await request + .post(`/svc/pubsub/emails/forward?token=${token}`) + .send(data) + .expect(200) + expect(res.text).to.eql('Newsletter') + }) + }) + + context('when email is an article', () => { + before(() => { + sinon.replace( + parser, + 'isProbablyNewsletter', + sinon.fake.resolves(false) + ) + sinon.replace(parser, 'isProbablyArticle', sinon.fake.resolves(true)) + }) + + it('saves the email as an article', async () => { + const data = { + message: { + data: Buffer.from( + JSON.stringify({ from, to, subject, html }) + ).toString('base64'), + publishTime: new Date().toISOString(), + }, + } + const res = await request + .post(`/svc/pubsub/emails/forward?token=${token}`) + .send(data) + .expect(200) + expect(res.text).to.eql('Article') + }) + }) + + context('when email is a regular email', () => { + before(() => { + sinon.replace( + parser, + 'isProbablyNewsletter', + sinon.fake.resolves(false) + ) + sinon.replace(parser, 'isProbablyArticle', sinon.fake.resolves(false)) + }) + + it('forwards the email', async () => { + const data = { + message: { + data: Buffer.from( + JSON.stringify({ from, to, subject, html }) + ).toString('base64'), + publishTime: new Date().toISOString(), + }, + } + const res = await request + .post(`/svc/pubsub/emails/forward?token=${token}`) + .send(data) + .expect(200) + expect(res.text).to.eql('Email forwarded') + }) + }) + }) +}) diff --git a/packages/api/test/utils/parser.test.ts b/packages/api/test/utils/parser.test.ts index 431933a1a..5c79e8619 100644 --- a/packages/api/test/utils/parser.test.ts +++ b/packages/api/test/utils/parser.test.ts @@ -5,12 +5,17 @@ import 'chai/register-should' import fs from 'fs' import { findNewsletterUrl, + generateUniqueUrl, + getTitleFromEmailSubject, + isProbablyArticle, isProbablyNewsletter, parsePageMetadata, parsePreparedContent, } from '../../src/utils/parser' import nock from 'nock' import chaiAsPromised from 'chai-as-promised' +import { User } from '../../src/entity/user' +import { createTestUser, deleteTestUser } from '../db' chai.use(chaiAsPromised) @@ -135,3 +140,42 @@ describe('parsePreparedContent', async () => { ) }) }) + +describe('isProbablyArticle', () => { + let user: User + + before(async () => { + user = await createTestUser('fakeUser') + }) + + after(async () => { + await deleteTestUser(user.name) + }) + + it('returns true when email is signed up with us', async () => { + const email = user.email + expect(await isProbablyArticle(email, 'test subject')).to.be.true + }) + + it('returns true when subject has omnivore: prefix', async () => { + const subject = 'omnivore: test subject' + expect(await isProbablyArticle('test-email', subject)).to.be.true + }) +}) + +describe('generateUniqueUrl', () => { + it('generates a unique URL', () => { + const url1 = generateUniqueUrl() + const url2 = generateUniqueUrl() + + expect(url1).to.not.eql(url2) + }) +}) + +describe('getTitleFromEmailSubject', () => { + it('returns the title from the email subject', () => { + const title = 'test subject' + const subject = `omnivore: ${title}` + expect(getTitleFromEmailSubject(subject)).to.eql(title) + }) +}) diff --git a/packages/inbound-email-handler/src/index.ts b/packages/inbound-email-handler/src/index.ts index 2e1d9022f..5f3b887b3 100644 --- a/packages/inbound-email-handler/src/index.ts +++ b/packages/inbound-email-handler/src/index.ts @@ -70,10 +70,10 @@ export const inboundEmailHandler = Sentry.GCPFunction.wrapHttpFunction( const html = parsed.html const text = parsed.text - const forwardedAddress = headers['x-forwarded-to'] - const recipientAddress = forwardedAddress?.toString() || parsed.to + const forwardedAddress = headers['x-forwarded-to']?.toString() + const recipientAddress = forwardedAddress || parsed.to const postHeader = headers['list-post']?.toString() - const unSubHeader = headers['list-unsubscribe'].toString() + const unSubHeader = headers['list-unsubscribe']?.toString() try { // check if it is a forwarding confirmation email or newsletter @@ -93,36 +93,43 @@ export const inboundEmailHandler = Sentry.GCPFunction.wrapHttpFunction( from, unSubHeader ) - } else { - console.log('non-newsletter email from:', from, recipientAddress) - - if (isConfirmationEmail(from)) { - console.log('handleConfirmation', from) - await handleConfirmation(recipientAddress, subject) - } else if (pdfAttachment) { - console.log('handle PDF attachment', from, recipientAddress) - await handlePdfAttachment( - recipientAddress, - pdfAttachmentName, - pdfAttachment, - subject - ) - } - - const unsubscribe = parseUnsubscribe(unSubHeader) - // queue non-newsletter emails - await pubsub.topic(NON_NEWSLETTER_EMAIL_TOPIC).publishMessage({ - json: { - from: from, - to: recipientAddress, - subject: subject, - html: html, - text: text, - unsubMailTo: unsubscribe.mailTo, - unsubHttpUrl: unsubscribe.httpUrl, - }, - }) + return res.send('ok') } + + console.log('non-newsletter email from:', from, recipientAddress) + + if (isConfirmationEmail(from)) { + console.log('handleConfirmation', from) + await handleConfirmation(recipientAddress, subject) + return res.send('ok') + } + + if (pdfAttachment) { + console.log('handle PDF attachment', from, recipientAddress) + await handlePdfAttachment( + recipientAddress, + pdfAttachmentName, + pdfAttachment, + subject + ) + return res.send('ok') + } + + const unsubscribe = parseUnsubscribe(unSubHeader) + // queue non-newsletter emails + await pubsub.topic(NON_NEWSLETTER_EMAIL_TOPIC).publishMessage({ + json: { + from, + to: recipientAddress, + subject, + html, + text, + unsubMailTo: unsubscribe.mailTo, + unsubHttpUrl: unsubscribe.httpUrl, + }, + }) + + res.send('ok') } catch (error) { console.log( 'error handling emails, will forward.', @@ -133,16 +140,14 @@ export const inboundEmailHandler = Sentry.GCPFunction.wrapHttpFunction( // queue error emails await pubsub.topic(NON_NEWSLETTER_EMAIL_TOPIC).publishMessage({ json: { - from: from, + from, to: recipientAddress, - subject: subject, - html: html, - text: text, + subject, + html, + text, }, }) } - - res.send('ok') } catch (e) { console.log(e) res.send(e)