From c3caf15e44a55f7156303f61642e8a504084e14a Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 11 Sep 2023 23:58:57 +0900 Subject: [PATCH] fix auth tests --- .../api/src/routers/svc/email_attachment.ts | 6 +-- packages/api/src/routers/svc/emails.ts | 8 ++-- packages/api/src/routers/svc/newsletters.ts | 4 +- packages/api/src/services/labels.ts | 40 +++++++++++-------- packages/api/src/services/library_item.ts | 40 ++++++++++--------- packages/api/src/services/newsletters.ts | 17 ++++---- packages/api/src/services/save_email.ts | 28 +++++++------ .../api/src/services/user_device_tokens.ts | 22 ++++++---- packages/api/src/services/webhook.ts | 2 +- .../api/test/resolvers/newsletters.test.ts | 7 ++-- .../test/resolvers/user_device_tokens.test.ts | 10 ++--- packages/api/test/routers/auth.test.ts | 2 +- packages/api/test/routers/emails.test.ts | 21 +++++----- .../api/test/routers/integrations.test.ts | 1 + 14 files changed, 115 insertions(+), 93 deletions(-) diff --git a/packages/api/src/routers/svc/email_attachment.ts b/packages/api/src/routers/svc/email_attachment.ts index 255657e79..c50b79d7a 100644 --- a/packages/api/src/routers/svc/email_attachment.ts +++ b/packages/api/src/routers/svc/email_attachment.ts @@ -10,7 +10,7 @@ import { env } from '../../env' import { UploadFileStatus } from '../../generated/graphql' import { authTrx } from '../../repository' import { createLibraryItem } from '../../services/library_item' -import { findNewsletterEmail } from '../../services/newsletters' +import { findNewsletterEmailByAddress } from '../../services/newsletters' import { updateReceivedEmail } from '../../services/received_emails' import { findUploadFileById, @@ -45,7 +45,7 @@ export function emailAttachmentRouter() { return res.status(401).send('UNAUTHORIZED') } - const newsletterEmail = await findNewsletterEmail(email) + const newsletterEmail = await findNewsletterEmailByAddress(email) if (!newsletterEmail || !newsletterEmail.user) { return res.status(401).send('UNAUTHORIZED') } @@ -112,7 +112,7 @@ export function emailAttachmentRouter() { return res.status(401).send('UNAUTHORIZED') } - const newsletterEmail = await findNewsletterEmail(email) + const newsletterEmail = await findNewsletterEmailByAddress(email) if (!newsletterEmail || !newsletterEmail.user) { return res.status(401).send('UNAUTHORIZED') } diff --git a/packages/api/src/routers/svc/emails.ts b/packages/api/src/routers/svc/emails.ts index 010b83494..3d66ee5c9 100644 --- a/packages/api/src/routers/svc/emails.ts +++ b/packages/api/src/routers/svc/emails.ts @@ -2,7 +2,7 @@ import cors from 'cors' import express from 'express' import { env } from '../../env' import { readPushSubscription } from '../../pubsub' -import { findNewsletterEmail } from '../../services/newsletters' +import { findNewsletterEmailByAddress } from '../../services/newsletters' import { saveReceivedEmail } from '../../services/received_emails' import { saveNewsletter } from '../../services/save_newsletter_email' import { analytics } from '../../utils/analytics' @@ -57,12 +57,12 @@ export function emailsServiceRouter() { const data = JSON.parse(message) as unknown if (!isEmailMessage(data)) { logger.error('Invalid message') - res.status(400).send('Bad Request') + res.status(200).send('Bad Request') return } // get user from newsletter email - const newsletterEmail = await findNewsletterEmail(data.to) + const newsletterEmail = await findNewsletterEmailByAddress(data.to) if (!newsletterEmail) { logger.info('newsletter email not found', { email: data.to }) @@ -150,7 +150,7 @@ export function emailsServiceRouter() { try { // get user from newsletter email - const newsletterEmail = await findNewsletterEmail(req.body.to) + const newsletterEmail = await findNewsletterEmailByAddress(req.body.to) if (!newsletterEmail) { logger.info('newsletter email not found', { email: req.body.to }) diff --git a/packages/api/src/routers/svc/newsletters.ts b/packages/api/src/routers/svc/newsletters.ts index 500f546db..6d16e1a9c 100644 --- a/packages/api/src/routers/svc/newsletters.ts +++ b/packages/api/src/routers/svc/newsletters.ts @@ -2,7 +2,7 @@ import express from 'express' import { SubscriptionStatus } from '../../generated/graphql' import { readPushSubscription } from '../../pubsub' import { - findNewsletterEmail, + findNewsletterEmailByAddress, updateConfirmationCode, } from '../../services/newsletters' import { updateReceivedEmail } from '../../services/received_emails' @@ -105,7 +105,7 @@ export function newsletterServiceRouter() { } // get user from newsletter email - const newsletterEmail = await findNewsletterEmail(data.email) + const newsletterEmail = await findNewsletterEmailByAddress(data.email) if (!newsletterEmail) { logger.info(`newsletter email not found: ${data.email}`) return res.status(200).send('Not Found') diff --git a/packages/api/src/services/labels.ts b/packages/api/src/services/labels.ts index fce6e9cd3..2197aecb0 100644 --- a/packages/api/src/services/labels.ts +++ b/packages/api/src/services/labels.ts @@ -26,26 +26,32 @@ export const findOrCreateLabels = async ( labels: CreateLabelInput[], userId: string ): Promise => { - return authTrx(async (tx) => { - const labelRepo = tx.withRepository(labelRepository) - // find existing labels - const labelEntities = await labelRepo.findByNames(labels.map((l) => l.name)) + return authTrx( + async (tx) => { + const labelRepo = tx.withRepository(labelRepository) + // find existing labels + const labelEntities = await labelRepo.findByNames( + labels.map((l) => l.name) + ) - const existingLabelsInLowerCase = labelEntities.map((l) => - l.name.toLowerCase() - ) - const newLabels = labels.filter( - (l) => !existingLabelsInLowerCase.includes(l.name.toLowerCase()) - ) - if (newLabels.length === 0) { - return labelEntities - } + const existingLabelsInLowerCase = labelEntities.map((l) => + l.name.toLowerCase() + ) + const newLabels = labels.filter( + (l) => !existingLabelsInLowerCase.includes(l.name.toLowerCase()) + ) + if (newLabels.length === 0) { + return labelEntities + } - // create new labels - const newLabelEntities = await labelRepo.createLabels(newLabels, userId) + // create new labels + const newLabelEntities = await labelRepo.createLabels(newLabels, userId) - return [...labelEntities, ...newLabelEntities] - }) + return [...labelEntities, ...newLabelEntities] + }, + undefined, + userId + ) } export const saveLabelsInLibraryItem = async ( diff --git a/packages/api/src/services/library_item.ts b/packages/api/src/services/library_item.ts index 7870f5c22..c74ad0236 100644 --- a/packages/api/src/services/library_item.ts +++ b/packages/api/src/services/library_item.ts @@ -276,28 +276,32 @@ export const searchLibraryItems = async ( const sortField = sort?.by || SortBy.SAVED // add pagination and sorting - return authTrx(async (tx) => { - const queryBuilder = tx - .createQueryBuilder(LibraryItem, 'library_item') - .leftJoinAndSelect('library_item.labels', 'labels') - .leftJoinAndSelect('library_item.highlights', 'highlights') - .leftJoinAndSelect('highlights.user', 'user') - .leftJoinAndSelect('user.profile', 'profile') - .where('library_item.user_id = :userId', { userId }) + return authTrx( + async (tx) => { + const queryBuilder = tx + .createQueryBuilder(LibraryItem, 'library_item') + .leftJoinAndSelect('library_item.labels', 'labels') + .leftJoinAndSelect('library_item.highlights', 'highlights') + .leftJoinAndSelect('highlights.user', 'user') + .leftJoinAndSelect('user.profile', 'profile') + .where('library_item.user_id = :userId', { userId }) - // build the where clause - buildWhereClause(queryBuilder, args) + // build the where clause + buildWhereClause(queryBuilder, args) - const libraryItems = await queryBuilder - .addOrderBy(`library_item.${sortField}`, sortOrder) - .offset(from) - .limit(size) - .getMany() + const libraryItems = await queryBuilder + .addOrderBy(`library_item.${sortField}`, sortOrder) + .offset(from) + .limit(size) + .getMany() - const count = await queryBuilder.getCount() + const count = await queryBuilder.getCount() - return { libraryItems, count } - }) + return { libraryItems, count } + }, + undefined, + userId + ) } export const findLibraryItemById = async ( diff --git a/packages/api/src/services/newsletters.ts b/packages/api/src/services/newsletters.ts index dd57b00e9..fb9e33cca 100644 --- a/packages/api/src/services/newsletters.ts +++ b/packages/api/src/services/newsletters.ts @@ -9,10 +9,10 @@ import { getRepository } from '../repository' import { userRepository } from '../repository/user' import addressparser = require('nodemailer/lib/addressparser') -const parsedAddress = (emailAddress: string): string | undefined => { +const parsedAddress = (emailAddress: string) => { const res = addressparser(emailAddress, { flatten: true }) if (!res || res.length < 1) { - return undefined + throw new Error('Invalid email address') } return res[0].address } @@ -72,7 +72,7 @@ export const updateConfirmationCode = async ( const address = parsedAddress(emailAddress) const result = await getRepository(NewsletterEmail) .createQueryBuilder() - .where('address ILIKE :address', { address }) + .where('LOWER(address) = :address', { address: address.toLowerCase() }) .update({ confirmationCode: confirmationCode, }) @@ -81,14 +81,14 @@ export const updateConfirmationCode = async ( return !!result.affected } -export const findNewsletterEmail = async ( +export const findNewsletterEmailByAddress = async ( emailAddress: string ): Promise => { const address = parsedAddress(emailAddress) return getRepository(NewsletterEmail) .createQueryBuilder('newsletter_email') .innerJoinAndSelect('newsletter_email.user', 'user') - .where('address ILIKE :address', { address }) + .where('LOWER(address) = :address', { address: address.toLowerCase() }) .getOne() } @@ -106,9 +106,8 @@ const createRandomEmailAddress = (userName: string, length: number): string => { return `${userName}-${nanoid(length)}e@${inbox}.omnivore.app` } -export const getNewsletterEmailById = async ( - id: string, - userId: string +export const findNewsletterEmailById = async ( + id: string ): Promise => { - return getRepository(NewsletterEmail).findOneBy({ id, user: { id: userId } }) + return getRepository(NewsletterEmail).findOneBy({ id }) } diff --git a/packages/api/src/services/save_email.ts b/packages/api/src/services/save_email.ts index f4b7ac4cc..0aa53a6dc 100644 --- a/packages/api/src/services/save_email.ts +++ b/packages/api/src/services/save_email.ts @@ -3,9 +3,7 @@ import { LibraryItemState, LibraryItemType, } from '../entity/library_item' -import { authTrx } from '../repository' import { getInternalLabelWithColor } from '../repository/label' -import { libraryItemRepository } from '../repository/library_item' import { enqueueThumbnailTask } from '../utils/createTask' import { cleanUrl, @@ -23,7 +21,11 @@ import { parseUrlMetadata, } from '../utils/parser' import { findOrCreateLabels } from './labels' -import { createLibraryItem } from './library_item' +import { + createLibraryItem, + findLibraryItemByUrl, + updateLibraryItem, +} from './library_item' import { updateReceivedEmail } from './received_emails' import { saveSubscription } from './subscriptions' @@ -69,17 +71,19 @@ export const saveEmail = async ( siteIcon = await fetchFavicon(url) } - const existingLibraryItem = await authTrx((t) => - t.withRepository(libraryItemRepository).findOneBy({ - originalUrl: cleanedUrl, - state: LibraryItemState.Succeeded, - }) + const existingLibraryItem = await findLibraryItemByUrl( + cleanedUrl, + input.userId ) if (existingLibraryItem) { - const updatedLibraryItem = await libraryItemRepository.save({ - ...existingLibraryItem, - archivedAt: null, - }) + const updatedLibraryItem = await updateLibraryItem( + existingLibraryItem.id, + { + archivedAt: null, + state: LibraryItemState.Succeeded, + }, + input.userId + ) logger.info('updated page from email', updatedLibraryItem) return updatedLibraryItem diff --git a/packages/api/src/services/user_device_tokens.ts b/packages/api/src/services/user_device_tokens.ts index 1f991899a..19c64e45c 100644 --- a/packages/api/src/services/user_device_tokens.ts +++ b/packages/api/src/services/user_device_tokens.ts @@ -5,9 +5,14 @@ import { authTrx } from '../repository' import { analytics } from '../utils/analytics' export const findDeviceTokenById = async ( - id: string + id: string, + userId?: string ): Promise => { - return authTrx((t) => t.getRepository(UserDeviceToken).findOneBy({ id })) + return authTrx( + (t) => t.getRepository(UserDeviceToken).findOneBy({ id }), + undefined, + userId + ) } export const findDeviceTokenByToken = async ( @@ -41,11 +46,14 @@ export const createDeviceToken = async ( }, }) - return authTrx((t) => - t.getRepository(UserDeviceToken).save({ - token, - user: { id: userId }, - }) + return authTrx( + (t) => + t.getRepository(UserDeviceToken).save({ + token, + user: { id: userId }, + }), + undefined, + userId ) } diff --git a/packages/api/src/services/webhook.ts b/packages/api/src/services/webhook.ts index 26b93152f..4747d225d 100644 --- a/packages/api/src/services/webhook.ts +++ b/packages/api/src/services/webhook.ts @@ -36,7 +36,7 @@ export const findWebhooks = async (userId?: string) => { export const findWebhookById = async (id: string, userId?: string) => { return authTrx( - (tx) => tx.getRepository(Webhook).findBy({ id }), + (tx) => tx.getRepository(Webhook).findOneBy({ id }), undefined, userId ) diff --git a/packages/api/test/resolvers/newsletters.test.ts b/packages/api/test/resolvers/newsletters.test.ts index 042ce26b6..8e4ea893a 100644 --- a/packages/api/test/resolvers/newsletters.test.ts +++ b/packages/api/test/resolvers/newsletters.test.ts @@ -9,7 +9,8 @@ import { import { getRepository } from '../../src/repository' import { createNewsletterEmail, - findNewsletterEmail, + findNewsletterEmailByAddress, + findNewsletterEmailById, } from '../../src/services/newsletters' import { createSubscription } from '../../src/services/subscriptions' import { deleteUser } from '../../src/services/user' @@ -181,7 +182,7 @@ describe('Newsletters API', () => { it('responds with status code 200', async () => { const response = await graphqlRequest(query, authToken).expect(200) - const newsletterEmail = await findNewsletterEmail( + const newsletterEmail = await findNewsletterEmailById( response.body.data.createNewsletterEmail.id ) expect(newsletterEmail).not.to.be.undefined @@ -241,7 +242,7 @@ describe('Newsletters API', () => { it('responds with status code 200', async () => { const response = await graphqlRequest(query, authToken).expect(200) - const newsletterEmail = await findNewsletterEmail( + const newsletterEmail = await findNewsletterEmailByAddress( response.body.data.deleteNewsletterEmail.newsletterEmail.id ) expect(newsletterEmail).to.be.null diff --git a/packages/api/test/resolvers/user_device_tokens.test.ts b/packages/api/test/resolvers/user_device_tokens.test.ts index 2191fff0d..1f22179e2 100644 --- a/packages/api/test/resolvers/user_device_tokens.test.ts +++ b/packages/api/test/resolvers/user_device_tokens.test.ts @@ -5,7 +5,7 @@ import { UserDeviceToken } from '../../src/entity/user_device_tokens' import { SetDeviceTokenErrorCode } from '../../src/generated/graphql' import { getRepository } from '../../src/repository' import { deleteUser } from '../../src/services/user' -import { deleteDeviceTokens, findDeviceTokenById } from '../../src/services/user_device_tokens' +import { createDeviceToken, deleteDeviceTokens, findDeviceTokenById } from '../../src/services/user_device_tokens' import { createTestDeviceToken, createTestUser } from '../db' import { generateFakeUuid, graphqlRequest, request } from '../util' @@ -106,7 +106,8 @@ describe('Device tokens API', () => { it('responds with status code 200 and creates the token', async () => { const response = await graphqlRequest(query, authToken).expect(200) const deviceToken = await findDeviceTokenById( - response.body.data.setDeviceToken.deviceToken.id + response.body.data.setDeviceToken.deviceToken.id, + user.id ) expect(deviceToken).not.to.be.null }) @@ -163,10 +164,7 @@ describe('Device tokens API', () => { before(async () => { // create test device token - await getRepository(UserDeviceToken).save({ - user: { id: user.id }, - token, - }) + await createDeviceToken(user.id, token) }) after(async () => { diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index d70b5a0f4..47d87737f 100644 --- a/packages/api/test/routers/auth.test.ts +++ b/packages/api/test/routers/auth.test.ts @@ -606,7 +606,7 @@ describe('auth router', () => { pendingUserToken!, 'web' ).expect(200) - const user = await getRepository(User).findOneByOrFail({ name }) + const user = await userRepository.findOneByOrFail({ name }) const { count } = await searchLibraryItems({}, user.id) expect(count).to.eql(3) diff --git a/packages/api/test/routers/emails.test.ts b/packages/api/test/routers/emails.test.ts index 175a3538e..18a181848 100644 --- a/packages/api/test/routers/emails.test.ts +++ b/packages/api/test/routers/emails.test.ts @@ -2,6 +2,7 @@ import { expect } from 'chai' import * as jwt from 'jsonwebtoken' import 'mocha' import sinon from 'sinon' +import { NewsletterEmail } from '../../src/entity/newsletter_email' import { ReceivedEmail } from '../../src/entity/received_email' import { User } from '../../src/entity/user' import { createNewsletterEmail } from '../../src/services/newsletters' @@ -14,31 +15,32 @@ import { createTestUser } from '../db' import { request } from '../util' describe('Emails Router', () => { - const newsletterEmail = 'fakeUser@omnivore.app' const from = 'fake from' const subject = 'fake subject' const text = 'fake text' - const to = newsletterEmail let user: User let token: string let receivedEmail: ReceivedEmail + let newsletterEmail: NewsletterEmail + let authToken: string before(async () => { // create test user and login user = await createTestUser('fakeUser') - await createNewsletterEmail(user.id, newsletterEmail) + newsletterEmail = await createNewsletterEmail(user.id) token = process.env.PUBSUB_VERIFICATION_TOKEN! receivedEmail = await saveReceivedEmail( from, - to, + newsletterEmail.address, subject, text, '', user.id, 'non-article' ) + authToken = jwt.sign(user.id, process.env.JWT_SECRET || '') }) after(async () => { @@ -74,7 +76,7 @@ describe('Emails Router', () => { data: Buffer.from( JSON.stringify({ from, - to, + to: newsletterEmail.address, subject, html, text, @@ -103,7 +105,7 @@ describe('Emails Router', () => { data: Buffer.from( JSON.stringify({ from, - to, + to: newsletterEmail.address, subject, html, text, @@ -128,14 +130,13 @@ describe('Emails Router', () => { const text = 'test text' const from = 'fake from' const subject = 'fake subject' - const authToken = jwt.sign(newsletterEmail, process.env.JWT_SECRET || '') it('saves the email in the database', async () => { const data = { html, text, from, - to: newsletterEmail, + to: newsletterEmail.address, subject, } const res = await request @@ -150,7 +151,7 @@ describe('Emails Router', () => { it('saves the email if body is empty', async () => { const data = { from, - to: newsletterEmail, + to: newsletterEmail.address, subject, } const res = await request @@ -165,7 +166,7 @@ describe('Emails Router', () => { it('saves the email if subject is empty', async () => { const data = { from, - to: newsletterEmail, + to: newsletterEmail.address, html, } const res = await request diff --git a/packages/api/test/routers/integrations.test.ts b/packages/api/test/routers/integrations.test.ts index 5d86923f2..7d3db5ab7 100644 --- a/packages/api/test/routers/integrations.test.ts +++ b/packages/api/test/routers/integrations.test.ts @@ -151,6 +151,7 @@ describe('Integrations routers', () => { shortId: 'test shortId', highlightPositionPercent, user, + libraryItem: item, }, item.id, user.id