Merge pull request #667 from auth0/DXCDT-240-blank-logo-url-validation

DXCDT-240: Prevent empty `logo_url` update payload inclusion

Author: willvedd

src/tools/auth0/handlers/branding.ts

@@ -69,13 +69,15 @@ export default class BrandingHandler extends DefaultHandler {
   }
 
   async processChanges(assets: Assets) {
-    // quit early if there's no branding to process.
     if (!assets.branding) return;
 
-    // remove templates, we only want top level branding settings for this API call
     const { templates, ...brandingSettings } = assets.branding;
 
-    // Do nothing if not set
+    if (brandingSettings.logo_url === '') {
+      //Sometimes blank logo_url returned by API but is invalid on import. See: DXCDT-240
+      delete brandingSettings.logo_url;
+    }
+
     if (brandingSettings && Object.keys(brandingSettings).length) {
       await this.client.branding.updateSettings({}, brandingSettings);
       this.updated += 1;

src/types.ts

@@ -208,9 +208,11 @@ export type Asset = { [key: string]: any };
 export type Assets = Partial<{
   actions: Action[] | null;
   attackProtection: Asset | null;
-  branding: {
-    templates?: { template: string; body: string }[] | null;
-  } | null;
+  branding:
+    | (Asset & {
+        templates?: { template: string; body: string }[] | null;
+      })
+    | null;
   clients: Asset[] | null;
   clientGrants: Asset[] | null;
   connections: Asset[] | null;

test/tools/auth0/handlers/branding.tests.js

@@ -156,6 +156,74 @@ describe('#branding handler', () => {
       ]);
     });
 
+    it('should ignore empty string `logo_url` during update', async () => {
+      let wasUpdateCalled = false;
+
+      const auth0 = {
+        branding: {
+          updateSettings: (_params, data) => {
+            expect(data).to.deep.equal({
+              colors: {
+                primary: '#F8F8F2',
+                page_background: '#112',
+              },
+              font: {
+                url: 'https://mycompany.org/font/myfont.ttf',
+              },
+            });
+            expect(data.logo_url).to.be.undefined; // eslint-disable-line no-unused-expressions
+            wasUpdateCalled = true;
+          },
+        },
+      };
+
+      const handler = new branding.default({ client: auth0 });
+      const stageFn = Object.getPrototypeOf(handler).processChanges;
+
+      await stageFn.apply(handler, [
+        {
+          branding: {
+            logo_url: '', // Note the empty string
+            colors: {
+              primary: '#F8F8F2',
+              page_background: '#112',
+            },
+            font: {
+              url: 'https://mycompany.org/font/myfont.ttf',
+            },
+          },
+        },
+      ]);
+
+      expect(wasUpdateCalled).to.equal(true);
+    });
+
+    it('should not send updateSettings request if empty object passed', async () => {
+      let wasUpdateCalled = false;
+
+      const auth0 = {
+        branding: {
+          updateSettings: () => {
+            wasUpdateCalled = true;
+            throw new Error(
+              'updateSettings should not have been called because omitted `logo_url` means that no API request needs to be made.'
+            );
+          },
+        },
+      };
+
+      const handler = new branding.default({ client: auth0 });
+      const stageFn = Object.getPrototypeOf(handler).processChanges;
+
+      await stageFn.apply(handler, [
+        {
+          branding: {},
+        },
+      ]);
+
+      expect(wasUpdateCalled).to.equal(false);
+    });
+
     it('should not throw, and be no-op if branding not set in context', async () => {
       const auth0 = {
         branding: {