From 20a80142d3d205784aa478cd5356512b45ff9c6d Mon Sep 17 00:00:00 2001 From: Mateusz Kwasniewski Date: Tue, 6 May 2025 19:08:04 +0200 Subject: [PATCH] feat: normalize urls in feature links (#9911) --- package.json | 1 + .../feature-link-service.test.ts | 59 +++++++++++++++---- .../feature-links/feature-link-service.ts | 35 ++++++++--- .../feature-links/feature-link.e2e.test.ts | 17 +++--- yarn.lock | 8 +++ 5 files changed, 94 insertions(+), 26 deletions(-) diff --git a/package.json b/package.json index 58b0912a5c..a500afd999 100644 --- a/package.json +++ b/package.json @@ -153,6 +153,7 @@ "murmurhash3js": "^3.0.1", "mustache": "^4.1.0", "nodemailer": "^6.9.9", + "normalize-url": "^6.1.0", "openapi-types": "^12.1.3", "owasp-password-strength-test": "^1.3.0", "parse-database-url": "^0.3.0", diff --git a/src/lib/features/feature-links/feature-link-service.test.ts b/src/lib/features/feature-links/feature-link-service.test.ts index b26dfc19c3..7efd1eb078 100644 --- a/src/lib/features/feature-links/feature-link-service.test.ts +++ b/src/lib/features/feature-links/feature-link-service.test.ts @@ -1,7 +1,7 @@ import { createFakeFeatureLinkService } from './createFeatureLinkService'; import type { IAuditUser, IUnleashConfig } from '../../types'; import getLogger from '../../../test/fixtures/no-logger'; -import { NotFoundError } from '../../error'; +import { BadDataError, NotFoundError } from '../../error'; test('create, update and delete feature link', async () => { const { featureLinkStore, featureLinkService } = @@ -16,18 +16,22 @@ test('create, update and delete feature link', async () => { ); expect(link).toMatchObject({ featureName: 'feature', - url: 'example.com', + url: 'https://example.com', title: 'some title', }); const newLink = await featureLinkService.updateLink( { projectId: 'default', linkId: link.id }, - { title: 'new title', url: 'example1.com', featureName: 'feature' }, + { + title: 'new title', + url: 'https://example1.com', + featureName: 'feature', + }, {} as IAuditUser, ); expect(newLink).toMatchObject({ featureName: 'feature', - url: 'example1.com', + url: 'https://example1.com', title: 'new title', }); @@ -39,22 +43,55 @@ test('create, update and delete feature link', async () => { }); test('cannot delete/update non existent link', async () => { - const { featureLinkStore, featureLinkService } = - createFakeFeatureLinkService({ - getLogger, - } as unknown as IUnleashConfig); + const { featureLinkService } = createFakeFeatureLinkService({ + getLogger, + } as unknown as IUnleashConfig); await expect( featureLinkService.updateLink( - { projectId: 'default', linkId: 'nonexitent' }, - { title: 'new title', url: 'example1.com', featureName: 'feature' }, + { projectId: 'default', linkId: 'nonexistent' }, + { + title: 'new title', + url: 'https://example1.com', + featureName: 'feature', + }, {} as IAuditUser, ), ).rejects.toThrow(NotFoundError); await expect( featureLinkService.deleteLink( - { projectId: 'default', linkId: 'nonexitent' }, + { projectId: 'default', linkId: 'nonexistent' }, {} as IAuditUser, ), ).rejects.toThrow(NotFoundError); }); + +test('cannot create/update invalid link', async () => { + const { featureLinkService } = createFakeFeatureLinkService({ + getLogger, + } as unknown as IUnleashConfig); + + await expect( + featureLinkService.createLink( + 'irrelevant', + { + featureName: 'irrelevant', + url: '%example.com', + title: 'irrelevant', + }, + {} as IAuditUser, + ), + ).rejects.toThrow(BadDataError); + + await expect( + featureLinkService.updateLink( + { projectId: 'irrelevant', linkId: 'irrelevant' }, + { + title: 'irrelevant', + url: '%example.com', + featureName: 'irrelevant', + }, + {} as IAuditUser, + ), + ).rejects.toThrow(BadDataError); +}); diff --git a/src/lib/features/feature-links/feature-link-service.ts b/src/lib/features/feature-links/feature-link-service.ts index 61a8c1246f..739abd5834 100644 --- a/src/lib/features/feature-links/feature-link-service.ts +++ b/src/lib/features/feature-links/feature-link-service.ts @@ -1,17 +1,18 @@ import type { Logger } from '../../logger'; import { - type IUnleashConfig, - type IAuditUser, FeatureLinkAddedEvent, - FeatureLinkUpdatedEvent, FeatureLinkRemovedEvent, + FeatureLinkUpdatedEvent, + type IAuditUser, + type IUnleashConfig, } from '../../types'; import type { IFeatureLink, IFeatureLinkStore, } from './feature-link-store-type'; import type EventService from '../events/event-service'; -import { NotFoundError } from '../../error'; +import { BadDataError, NotFoundError } from '../../error'; +import normalizeUrl from 'normalize-url'; interface IFeatureLinkStoreObj { featureLinkStore: IFeatureLinkStore; @@ -36,18 +37,31 @@ export default class FeatureLinkService { return this.featureLinkStore.getAll(); } + private normalize(url: string) { + try { + return normalizeUrl(url, { defaultProtocol: 'https:' }); + } catch (e) { + throw new BadDataError(`Invalid URL: ${url}`); + } + } + async createLink( projectId: string, newLink: Omit, auditUser: IAuditUser, ): Promise { - const link = await this.featureLinkStore.insert(newLink); + const normalizedUrl = this.normalize(newLink.url); + + const link = await this.featureLinkStore.insert({ + ...newLink, + url: normalizedUrl, + }); await this.eventService.storeEvent( new FeatureLinkAddedEvent({ featureName: newLink.featureName, project: projectId, - data: { url: newLink.url, title: newLink.title }, + data: { url: normalizedUrl, title: newLink.title }, auditUser, }), ); @@ -60,19 +74,24 @@ export default class FeatureLinkService { updatedLink: Omit, auditUser: IAuditUser, ): Promise { + const normalizedUrl = this.normalize(updatedLink.url); + const preData = await this.featureLinkStore.get(linkId); if (!preData) { throw new NotFoundError(`Could not find link with id ${linkId}`); } - const link = await this.featureLinkStore.update(linkId, updatedLink); + const link = await this.featureLinkStore.update(linkId, { + ...updatedLink, + url: normalizedUrl, + }); await this.eventService.storeEvent( new FeatureLinkUpdatedEvent({ featureName: updatedLink.featureName, project: projectId, - data: { url: link.url, title: link.title }, + data: { url: normalizedUrl, title: link.title }, preData: { url: preData.url, title: preData.title }, auditUser, }), diff --git a/src/lib/features/feature-links/feature-link.e2e.test.ts b/src/lib/features/feature-links/feature-link.e2e.test.ts index 32a976b63d..70f6a001e1 100644 --- a/src/lib/features/feature-links/feature-link.e2e.test.ts +++ b/src/lib/features/feature-links/feature-link.e2e.test.ts @@ -92,14 +92,14 @@ test('should manage feature links', async () => { const links = await featureLinkStore.getAll(); expect(links).toMatchObject([ { - url: 'example.com', + url: 'https://example.com', title: 'feature link', featureName: 'my_feature', }, ]); const { body } = await app.getProjectFeatures('default', 'my_feature'); expect(body.links).toMatchObject([ - { id: links[0].id, title: 'feature link', url: 'example.com' }, + { id: links[0].id, title: 'feature link', url: 'https://example.com' }, ]); await updatedLink('my_feature', links[0].id, { @@ -110,7 +110,7 @@ test('should manage feature links', async () => { const updatedLinks = await featureLinkStore.getAll(); expect(updatedLinks).toMatchObject([ { - url: 'example_updated.com', + url: 'https://example_updated.com', title: 'feature link updated', featureName: 'my_feature', }, @@ -127,7 +127,7 @@ test('should manage feature links', async () => { type: 'feature-link-removed', data: null, preData: { - url: 'example_updated.com', + url: 'https://example_updated.com', title: 'feature link updated', }, featureName: 'my_feature', @@ -135,14 +135,17 @@ test('should manage feature links', async () => { }, { type: 'feature-link-updated', - data: { url: 'example_updated.com', title: 'feature link updated' }, - preData: { url: 'example.com', title: 'feature link' }, + data: { + url: 'https://example_updated.com', + title: 'feature link updated', + }, + preData: { url: 'https://example.com', title: 'feature link' }, featureName: 'my_feature', project: 'default', }, { type: 'feature-link-added', - data: { url: 'example.com', title: 'feature link' }, + data: { url: 'https://example.com', title: 'feature link' }, preData: null, featureName: 'my_feature', project: 'default', diff --git a/yarn.lock b/yarn.lock index 534f107e44..07c10d925a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7100,6 +7100,13 @@ __metadata: languageName: node linkType: hard +"normalize-url@npm:^6.1.0": + version: 6.1.0 + resolution: "normalize-url@npm:6.1.0" + checksum: 10c0/95d948f9bdd2cfde91aa786d1816ae40f8262946e13700bf6628105994fe0ff361662c20af3961161c38a119dc977adeb41fc0b41b1745eb77edaaf9cb22db23 + languageName: node + linkType: hard + "npm-run-path@npm:^4.0.1": version: 4.0.1 resolution: "npm-run-path@npm:4.0.1" @@ -9457,6 +9464,7 @@ __metadata: mustache: "npm:^4.1.0" nock: "npm:13.5.6" nodemailer: "npm:^6.9.9" + normalize-url: "npm:^6.1.0" openapi-enforcer: "npm:1.23.0" openapi-types: "npm:^12.1.3" owasp-password-strength-test: "npm:^1.3.0"