diff --git a/packages/api/src/repository/label.ts b/packages/api/src/repository/label.ts index baaac24bb..0faebd3db 100644 --- a/packages/api/src/repository/label.ts +++ b/packages/api/src/repository/label.ts @@ -46,9 +46,12 @@ export const labelRepository = appDataSource.getRepository(Label).extend({ return this.findOneBy({ id }) }, - findByName(name: string) { + findByName(name: string, userId: string) { return this.createQueryBuilder() - .where('LOWER(name) = LOWER(:name)', { name }) // case insensitive + .where('user_id = :userId AND LOWER(name) = LOWER(:name)', { + name, + userId, + }) // case insensitive .getOne() }, diff --git a/packages/api/src/resolvers/highlight/index.ts b/packages/api/src/resolvers/highlight/index.ts index 8e0f22c9e..a142b3299 100644 --- a/packages/api/src/resolvers/highlight/index.ts +++ b/packages/api/src/resolvers/highlight/index.ts @@ -81,7 +81,7 @@ export const mergeHighlightResolver = authorized< MergeHighlightSuccess, MergeHighlightError, MutationMergeHighlightArgs ->(async (_, { input }, { log, pubsub, uid }) => { +>(async (_, { input }, { authTrx, log, pubsub, uid }) => { const { overlapHighlightIdList, ...newHighlightInput } = input /* Compute merged annotation form the order of highlights appearing on page */ @@ -90,9 +90,10 @@ export const mergeHighlightResolver = authorized< const mergedColors: string[] = [] try { - const existingHighlights = await highlightRepository.findByLibraryItemId( - input.articleId, - uid + const existingHighlights = await authTrx((tx) => + tx + .withRepository(highlightRepository) + .findByLibraryItemId(input.articleId, uid) ) existingHighlights.forEach((highlight) => { diff --git a/packages/api/src/resolvers/labels/index.ts b/packages/api/src/resolvers/labels/index.ts index 1da2f3053..2e3b9c6d2 100644 --- a/packages/api/src/resolvers/labels/index.ts +++ b/packages/api/src/resolvers/labels/index.ts @@ -90,17 +90,22 @@ export const createLabelResolver = authorized< CreateLabelError, MutationCreateLabelArgs >(async (_, { input }, { authTrx, uid }) => { - const existingLabel = await labelRepository.findByName(input.name) - if (existingLabel) { + const label = await authTrx(async (tx) => { + const repo = tx.withRepository(labelRepository) + const existingLabel = await repo.findByName(input.name, uid) + if (existingLabel) { + return null + } + + return repo.createLabel(input, uid) + }) + + if (!label) { return { errorCodes: [CreateLabelErrorCode.LabelAlreadyExists], } } - const label = await authTrx(async (tx) => - tx.withRepository(labelRepository).createLabel(input, uid) - ) - analytics.capture({ distinctId: uid, event: 'label_created', diff --git a/packages/api/test/resolvers/highlight.test.ts b/packages/api/test/resolvers/highlight.test.ts index a81602cd1..aef7aecae 100644 --- a/packages/api/test/resolvers/highlight.test.ts +++ b/packages/api/test/resolvers/highlight.test.ts @@ -1,10 +1,9 @@ +/* eslint-disable @typescript-eslint/restrict-template-expressions */ import * as chai from 'chai' import { expect } from 'chai' import chaiString from 'chai-string' import 'mocha' -import { Highlight } from '../../src/entity/highlight' import { User } from '../../src/entity/user' -import { getRepository } from '../../src/repository' import { createHighlight, deleteHighlightById, @@ -157,7 +156,7 @@ describe('Highlights API', () => { .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) - authToken = res.body.authToken + authToken = res.body.authToken as string itemId = (await createTestLibraryItem(user.id)).id }) diff --git a/packages/api/test/resolvers/labels.test.ts b/packages/api/test/resolvers/labels.test.ts index 8d1175c78..0dccc54d1 100644 --- a/packages/api/test/resolvers/labels.test.ts +++ b/packages/api/test/resolvers/labels.test.ts @@ -145,7 +145,7 @@ describe('Labels API', () => { }) }) - context('when name exists', () => { + context('when name exists in the user library', () => { let existingLabel: Label before(async () => { @@ -177,6 +177,32 @@ describe('Labels API', () => { }) }) + context('when name exists in the other user library', () => { + let existingLabel: Label + let otherUser: User + + before(async () => { + otherUser = await createTestUser('otherUser') + existingLabel = await createLabel('label3', '#ffffff', otherUser.id) + }) + + after(async () => { + // delete other user will also delete the label + await deleteUser(otherUser.id) + }) + + it('creates the label', async () => { + const res = await graphqlRequest(query, authToken, { + input: { name: existingLabel.name }, + }).expect(200) + const label = await findLabelById( + res.body.data.createLabel.label.id, + user.id + ) + expect(label).to.exist + }) + }) + it('responds status code 400 when invalid query', async () => { const invalidQuery = ` mutation { diff --git a/packages/db/migrations/0173.do.alter_labels_table_policy.sql b/packages/db/migrations/0173.do.alter_labels_table_policy.sql new file mode 100755 index 000000000..1e7f4afdb --- /dev/null +++ b/packages/db/migrations/0173.do.alter_labels_table_policy.sql @@ -0,0 +1,11 @@ +-- Type: DO +-- Name: alter_labels_table_policy +-- Description: Alter labels table select policy to check user_id + +BEGIN; + +ALTER POLICY read_labels ON omnivore.labels + TO omnivore_user + USING (user_id = omnivore.get_current_user_id()); + +COMMIT; diff --git a/packages/db/migrations/0173.undo.alter_labels_table_policy.sql b/packages/db/migrations/0173.undo.alter_labels_table_policy.sql new file mode 100755 index 000000000..552a9e2c7 --- /dev/null +++ b/packages/db/migrations/0173.undo.alter_labels_table_policy.sql @@ -0,0 +1,11 @@ +-- Type: UNDO +-- Name: alter_labels_table_policy +-- Description: Alter labels table select policy to check user_id + +BEGIN; + +ALTER POLICY read_labels ON omnivore.labels + TO omnivore_user + USING (true); + +COMMIT;