feat: normalize urls in feature links (#9911)

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",

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 } =
+    const { featureLinkService } = createFakeFeatureLinkService({
-        createFakeFeatureLinkService({
         getLogger,
     } as unknown as IUnleashConfig);
 
     await expect(
         featureLinkService.updateLink(
-            { projectId: 'default', linkId: 'nonexitent' },
+            { projectId: 'default', linkId: 'nonexistent' },
-            { title: 'new title', url: 'example1.com', featureName: 'feature' },
+            {
+                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);
+});

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<IFeatureLink, 'id'>,
         auditUser: IAuditUser,
     ): Promise<IFeatureLink> {
-        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<IFeatureLink, 'id'>,
         auditUser: IAuditUser,
     ): Promise<IFeatureLink> {
+        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,
             }),

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' },
+            data: {
-            preData: { url: 'example.com', title: 'feature link' },
+                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',

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"