From 024d558cfe2a54564367066daf0b26a3c75e7787 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 7 Sep 2023 23:28:24 +0800 Subject: [PATCH] update labels rls --- packages/api/src/resolvers/highlight/index.ts | 6 +- packages/api/src/resolvers/webhooks/index.ts | 16 ++---- packages/api/src/services/highlights.ts | 7 ++- packages/api/src/services/labels.ts | 20 +++++++ packages/api/src/services/library_item.ts | 9 ++- packages/api/src/services/popular_reads.ts | 3 +- packages/api/src/services/rules.ts | 4 +- packages/api/src/services/webhook.ts | 13 +++++ packages/api/test/resolvers/labels.test.ts | 56 ++++++++----------- .../api/test/resolvers/popular_reads.test.ts | 8 +-- .../api/test/resolvers/recent_emails.test.ts | 5 +- packages/db/migrations/0123.do.add_rls.sql | 4 ++ packages/db/migrations/0123.undo.add_rls.sql | 12 ++++ 13 files changed, 104 insertions(+), 59 deletions(-) diff --git a/packages/api/src/resolvers/highlight/index.ts b/packages/api/src/resolvers/highlight/index.ts index 41b7c8fa2..a0919fdc8 100644 --- a/packages/api/src/resolvers/highlight/index.ts +++ b/packages/api/src/resolvers/highlight/index.ts @@ -178,9 +178,9 @@ export const updateHighlightResolver = authorized< const updatedHighlight = await updateHighlight( input.highlightId, { - annotation: input.annotation, - html: input.html, - quote: input.quote, + annotation: input.annotation ?? undefined, + html: input.html ?? undefined, + quote: input.quote ?? undefined, }, uid, pubsub diff --git a/packages/api/src/resolvers/webhooks/index.ts b/packages/api/src/resolvers/webhooks/index.ts index 664a7c9ab..606debe1e 100644 --- a/packages/api/src/resolvers/webhooks/index.ts +++ b/packages/api/src/resolvers/webhooks/index.ts @@ -20,6 +20,7 @@ import { WebhookSuccess, } from '../../generated/graphql' import { authTrx } from '../../repository' +import { deleteWebhook } from '../../services/webhook' import { analytics } from '../../utils/analytics' import { authorized } from '../../utils/helpers' @@ -80,9 +81,9 @@ export const deleteWebhookResolver = authorized< DeleteWebhookSuccess, DeleteWebhookError, MutationDeleteWebhookArgs ->(async (_, { id }, { authTrx, uid, log }) => { +>(async (_, { id }, { uid, log }) => { try { - await authTrx(async (t) => t.getRepository(Webhook).delete(id)) + const webhook = await deleteWebhook(id, uid) analytics.track({ userId: uid, @@ -94,16 +95,7 @@ export const deleteWebhookResolver = authorized< }) return { - webhook: { - id, - url: '', - eventTypes: [], - method: 'POST', - contentType: 'application/json', - enabled: false, - createdAt: new Date(), - updatedAt: new Date(), - }, + webhook: webhookDataToResponse(webhook), } } catch (error) { log.error('Error deleting webhook', error) diff --git a/packages/api/src/services/highlights.ts b/packages/api/src/services/highlights.ts index 0eca20b75..422b24d7e 100644 --- a/packages/api/src/services/highlights.ts +++ b/packages/api/src/services/highlights.ts @@ -76,8 +76,11 @@ export const updateHighlight = async ( const highlightRepo = tx.withRepository(highlightRepository) await highlightRepo.updateAndSave(highlightId, highlight) - return highlightRepo.findOneByOrFail({ - id: highlightId, + return highlightRepo.findOneOrFail({ + where: { id: highlightId }, + relations: { + libraryItem: true, + }, }) }) diff --git a/packages/api/src/services/labels.ts b/packages/api/src/services/labels.ts index fa6eb8dd5..fce6e9cd3 100644 --- a/packages/api/src/services/labels.ts +++ b/packages/api/src/services/labels.ts @@ -190,3 +190,23 @@ export const updateLabel = async ( userId ) } + +export const findLabelsByUserId = async (userId: string): Promise => { + return authTrx( + async (tx) => + tx.withRepository(labelRepository).find({ + where: { user: { id: userId } }, + order: { position: 'ASC' }, + }), + undefined, + userId + ) +} + +export const findLabelById = async (id: string, userId: string) => { + return authTrx( + async (tx) => tx.withRepository(labelRepository).findOneBy({ id }), + undefined, + userId + ) +} diff --git a/packages/api/src/services/library_item.ts b/packages/api/src/services/library_item.ts index 94de6dbe0..7870f5c22 100644 --- a/packages/api/src/services/library_item.ts +++ b/packages/api/src/services/library_item.ts @@ -12,6 +12,7 @@ import { BulkActionType } from '../generated/graphql' import { createPubSubClient, EntityType } from '../pubsub' import { authTrx } from '../repository' import { libraryItemRepository } from '../repository/library_item' +import { wordsCount } from '../utils/helpers' import { DateFilter, FieldFilter, @@ -387,7 +388,13 @@ export const createLibraryItem = async ( pubsub = createPubSubClient() ): Promise => { const newLibraryItem = await authTrx( - async (tx) => tx.withRepository(libraryItemRepository).save(libraryItem), + async (tx) => + tx.withRepository(libraryItemRepository).save({ + ...libraryItem, + wordCount: + libraryItem.wordCount ?? + wordsCount(libraryItem.readableContent || ''), + }), undefined, userId ) diff --git a/packages/api/src/services/popular_reads.ts b/packages/api/src/services/popular_reads.ts index 371a812bf..c030a877f 100644 --- a/packages/api/src/services/popular_reads.ts +++ b/packages/api/src/services/popular_reads.ts @@ -5,7 +5,7 @@ import { DeepPartial, EntityManager } from 'typeorm' import { LibraryItem, LibraryItemType } from '../entity/library_item' import { authTrx, entityManager } from '../repository' import { libraryItemRepository } from '../repository/library_item' -import { generateSlug, stringToHash } from '../utils/helpers' +import { generateSlug, stringToHash, wordsCount } from '../utils/helpers' import { logger } from '../utils/logger' import { createLibraryItem } from './library_item' @@ -75,6 +75,7 @@ const popularReadToLibraryItem = ( publishedAt: pr.publishedAt, siteName: pr.siteName, user: { id: userId }, + wordCount: wordsCount(pr.content), } } diff --git a/packages/api/src/services/rules.ts b/packages/api/src/services/rules.ts index e33024755..4153fe493 100644 --- a/packages/api/src/services/rules.ts +++ b/packages/api/src/services/rules.ts @@ -36,9 +36,9 @@ export const deleteRule = async (id: string, userId?: string) => { return authTrx( async (t) => { const repo = t.getRepository(Rule) + const rule = await repo.findOneByOrFail({ id }) await repo.delete(id) - - return repo.findOneByOrFail({ id }) + return rule }, undefined, userId diff --git a/packages/api/src/services/webhook.ts b/packages/api/src/services/webhook.ts index 40152adae..26b93152f 100644 --- a/packages/api/src/services/webhook.ts +++ b/packages/api/src/services/webhook.ts @@ -41,3 +41,16 @@ export const findWebhookById = async (id: string, userId?: string) => { userId ) } + +export const deleteWebhook = async (id: string, userId?: string) => { + return authTrx( + async (tx) => { + const repo = tx.getRepository(Webhook) + const webhook = await repo.findOneByOrFail({ id }) + await repo.delete(id) + return webhook + }, + undefined, + userId + ) +} diff --git a/packages/api/test/resolvers/labels.test.ts b/packages/api/test/resolvers/labels.test.ts index 249c2b6c9..4ca69ca07 100644 --- a/packages/api/test/resolvers/labels.test.ts +++ b/packages/api/test/resolvers/labels.test.ts @@ -5,12 +5,17 @@ import { Highlight } from '../../src/entity/highlight' import { Label } from '../../src/entity/label' import { LibraryItem } from '../../src/entity/library_item' import { User } from '../../src/entity/user' -import { getRepository } from '../../src/repository' import { createHighlight, findHighlightById, } from '../../src/services/highlights' -import { createLabel, deleteLabels, saveLabelsInHighlight } from '../../src/services/labels' +import { + createLabel, + deleteLabels, + findLabelById, + findLabelsByUserId, + saveLabelsInHighlight, +} from '../../src/services/labels' import { deleteLibraryItemById, findLibraryItemById, @@ -38,22 +43,17 @@ describe('Labels API', () => { }) describe('GET labels', () => { - let labels: Label[] let query: string before(async () => { // create testing labels - const label1 = await createLabel('label_1', '#ffffff', user.id) - const label2 = await createLabel('label_2', '#eeeeee', user.id) - labels = [label1, label2] + await createLabel('label_1', '#ffffff', user.id) + await createLabel('label_2', '#eeeeee', user.id) }) after(async () => { // clean up - await deleteLabels( - labels.map((l) => l.id), - user.id - ) + await deleteLabels({ user: { id: user.id } }, user.id) }) beforeEach(() => { @@ -80,9 +80,7 @@ describe('Labels API', () => { it('should return labels', async () => { const res = await graphqlRequest(query, authToken).expect(200) - const labels = await getRepository(Label).findBy({ - user: { id: user.id }, - }) + const labels = await findLabelsByUserId(user.id) expect(res.body.data.labels.labels).to.eql( labels.map((label) => ({ id: label.id, @@ -148,9 +146,10 @@ describe('Labels API', () => { it('should create label', async () => { const res = await graphqlRequest(query, authToken).expect(200) - const label = await getRepository(Label).findOneBy({ - id: res.body.data.createLabel.label.id, - }) + const label = await findLabelById( + res.body.data.createLabel.label.id, + user.id + ) expect(label).to.exist }) }) @@ -164,15 +163,13 @@ describe('Labels API', () => { }) after(async () => { - await deleteLabels([existingLabel.id], user.id) + await deleteLabels({ id: existingLabel.id }, user.id) }) it('should return error code BAD_REQUEST', async () => { const res = await graphqlRequest(query, authToken).expect(200) - expect(res.body.data.createLabel.errorCodes).to.eql([ - 'BAD_REQUEST', - ]) + expect(res.body.data.createLabel.errorCodes).to.eql(['BAD_REQUEST']) }) }) @@ -228,7 +225,7 @@ describe('Labels API', () => { it('should delete label', async () => { await graphqlRequest(query, authToken).expect(200) - const label = await getRepository(Label).findOneBy({ id: labelId }) + const label = await findLabelById(labelId, user.id) expect(label).not.exist }) }) @@ -335,7 +332,7 @@ describe('Labels API', () => { after(async () => { // clean up await deleteLabels( - labels.map((l) => l.id), + { user: { id: user.id } }, user.id ) await deleteLibraryItemById(item.id) @@ -461,7 +458,7 @@ describe('Labels API', () => { }) after(async () => { - await deleteLabels([toUpdateLabel.id], user.id) + await deleteLabels( { id: toUpdateLabel.id }, user.id) }) it('should return the updated label', async () => { @@ -475,9 +472,7 @@ describe('Labels API', () => { it('should update the label in db', async () => { await graphqlRequest(query, authToken).expect(200) - const updatedLabel = await getRepository(Label).findOne({ - where: { id: labelId }, - }) + const updatedLabel = await findLabelById(labelId, user.id) expect(updatedLabel?.name).to.eql(name) expect(updatedLabel?.color).to.eql(color) @@ -538,7 +533,7 @@ describe('Labels API', () => { after(async () => { // clean up await deleteLabels( - labels.map((l) => l.id), + { user: { id: user.id } }, user.id ) await deleteLibraryItemById(item.id) @@ -689,12 +684,7 @@ describe('Labels API', () => { expect(res.body.data.moveLabel.label.position).to.eql( labels[4].position ) - const reorderedLabels = await getRepository(Label).find({ - where: { - user: { id: user.id }, - }, - order: { position: 'ASC' }, - }) + const reorderedLabels = await findLabelsByUserId(user.id) expect(reorderedLabels.map((l) => l.id)).to.eql([ labels[0].id, labels[2].id, diff --git a/packages/api/test/resolvers/popular_reads.test.ts b/packages/api/test/resolvers/popular_reads.test.ts index d7d83474c..2f18a5cb9 100644 --- a/packages/api/test/resolvers/popular_reads.test.ts +++ b/packages/api/test/resolvers/popular_reads.test.ts @@ -42,7 +42,7 @@ describe('PopularReads API', () => { describe('addPopularRead', () => { it('should add a new article if the readName is valid', async () => { - const readName = 'omnivore_get_started' + const readName = 'omnivore_ios' const res = await graphqlRequest( addPopularReadQuery(readName), authToken @@ -54,14 +54,14 @@ describe('PopularReads API', () => { user.id ) expect(item?.originalUrl).to.eq( - 'https://blog.omnivore.app/p/getting-started-with-omnivore' + 'https://blog.omnivore.app/p/saving-links-from-your-iphone-or' ) - expect(item?.wordCount).to.eq(1155) + expect(item?.wordCount).to.eq(371) }) it('responds status code 500 when invalid user', async () => { const invalidAuthToken = 'Fake token' - const readName = 'omnivore_get_started' + const readName = 'omnivore_web' return graphqlRequest( addPopularReadQuery(readName), invalidAuthToken diff --git a/packages/api/test/resolvers/recent_emails.test.ts b/packages/api/test/resolvers/recent_emails.test.ts index 422f27a37..cbcd73fda 100644 --- a/packages/api/test/resolvers/recent_emails.test.ts +++ b/packages/api/test/resolvers/recent_emails.test.ts @@ -146,7 +146,10 @@ describe('Recent Emails Resolver', () => { expect(resp.body.data.markEmailAsItem.success).to.be.true - const updatedRecentEmail = await findReceivedEmailById(recentEmail.id) + const updatedRecentEmail = await findReceivedEmailById( + recentEmail.id, + user.id + ) expect(updatedRecentEmail?.type).to.eql('article') }) }) diff --git a/packages/db/migrations/0123.do.add_rls.sql b/packages/db/migrations/0123.do.add_rls.sql index 419b6a1de..ccc43d317 100755 --- a/packages/db/migrations/0123.do.add_rls.sql +++ b/packages/db/migrations/0123.do.add_rls.sql @@ -16,6 +16,8 @@ CREATE POLICY integrations_policy on omnivore.integrations WITH CHECK (user_id = omnivore.get_current_user_id()); GRANT SELECT, INSERT, UPDATE, DELETE ON omnivore.integrations TO omnivore_user; +DROP POLICY read_labels ON omnivore.labels; +DROP POLICY create_labels ON omnivore.labels; CREATE POLICY labels_policy on omnivore.labels USING (user_id = omnivore.get_current_user_id()) WITH CHECK (user_id = omnivore.get_current_user_id()); @@ -38,6 +40,8 @@ CREATE POLICY webhooks_policy on omnivore.webhooks WITH CHECK (user_id = omnivore.get_current_user_id()); GRANT SELECT, INSERT, UPDATE, DELETE ON omnivore.webhooks TO omnivore_user; +DROP POLICY read_user_device_tokens ON omnivore.user_device_tokens; +DROP POLICY create_user_device_tokens ON omnivore.user_device_tokens; CREATE POLICY user_device_tokens_policy on omnivore.user_device_tokens USING (user_id = omnivore.get_current_user_id()) WITH CHECK (user_id = omnivore.get_current_user_id()); diff --git a/packages/db/migrations/0123.undo.add_rls.sql b/packages/db/migrations/0123.undo.add_rls.sql index 6b17c40b8..544166c9a 100755 --- a/packages/db/migrations/0123.undo.add_rls.sql +++ b/packages/db/migrations/0123.undo.add_rls.sql @@ -11,6 +11,12 @@ ALTER TABLE omnivore.integrations DISABLE ROW LEVEL SECURITY; DROP POLICY integrations_policy on omnivore.integrations; DROP POLICY labels_policy on omnivore.labels; +CREATE POLICY read_labels on omnivore.labels + FOR SELECT TO omnivore_user + USING (true); +CREATE POLICY create_labels on omnivore.labels + FOR INSERT TO omnivore_user + WITH CHECK (true); ALTER TABLE omnivore.received_emails DISABLE ROW LEVEL SECURITY; DROP POLICY received_emails_policy on omnivore.received_emails; @@ -22,5 +28,11 @@ ALTER TABLE omnivore.webhooks DISABLE ROW LEVEL SECURITY; DROP POLICY webhooks_policy on omnivore.webhooks; DROP POLICY user_device_tokens_policy on omnivore.user_device_tokens; +CREATE POLICY read_user_device_tokens on omnivore.user_device_tokens + FOR SELECT TO omnivore_user + USING (true); +CREATE POLICY create_user_device_tokens on omnivore.user_device_tokens + FOR INSERT TO omnivore_user + WITH CHECK (true); COMMIT;