From a97fdaa8b32edae8569b49d6daaa279080a0806f Mon Sep 17 00:00:00 2001 From: Will Vedder Date: Fri, 24 Jun 2022 12:10:36 -0400 Subject: [PATCH 1/5] Fixing types --- src/tools/auth0/handlers/tenant.ts | 58 +++++++++++++++- src/types.ts | 8 +-- test/tools/auth0/handlers/tenant.tests.js | 84 +++++++++++++++++++++++ 3 files changed, 145 insertions(+), 5 deletions(-) diff --git a/src/tools/auth0/handlers/tenant.ts b/src/tools/auth0/handlers/tenant.ts index b5cd144bb..7da54b6d9 100644 --- a/src/tools/auth0/handlers/tenant.ts +++ b/src/tools/auth0/handlers/tenant.ts @@ -3,12 +3,14 @@ import ValidationError from '../../validationError'; import DefaultHandler, { order } from './default'; import { supportedPages, pageNameMap } from './pages'; import { convertJsonToString } from '../../utils'; -import { Asset, Assets } from '../../../types'; +import { Asset, Assets, Language } from '../../../types'; export const schema = { type: 'object', }; +export type Tenant = Asset & { enabled_locales: Language[]; flags: { [key: string]: boolean } }; + const blockPageKeys = [ ...Object.keys(pageNameMap), ...Object.values(pageNameMap), @@ -16,6 +18,8 @@ const blockPageKeys = [ ]; export default class TenantHandler extends DefaultHandler { + existing: Tenant; + constructor(options: DefaultHandler) { super({ ...options, @@ -25,6 +29,9 @@ export default class TenantHandler extends DefaultHandler { async getType(): Promise { const tenant = await this.client.tenant.getSettings(); + + this.existing = tenant; + blockPageKeys.forEach((key) => { if (tenant[key]) delete tenant[key]; }); @@ -53,6 +60,13 @@ export default class TenantHandler extends DefaultHandler { async processChanges(assets: Assets): Promise { const { tenant } = assets; + // Do nothing if not set + if (!tenant) return; + + const existingTenant = this.existing || this.getType(); + + const sanitizedFlags = removeUnapplicableMigrationFlags(tenant.flags, existingTenant.flags); + if (tenant && Object.keys(tenant).length > 0) { await this.client.tenant.updateSettings(tenant); this.updated += 1; @@ -60,3 +74,45 @@ export default class TenantHandler extends DefaultHandler { } } } + +export const removeUnapplicableMigrationFlags = ( + existingFlags: Tenant['flags'] = {}, + proposedFlags: Tenant['flags'] = {} +): Tenant['flags'] => { + /* + Tenants can only update migration flags that are already configured. + If moving configuration from one tenant to another, there may be instances + where different migration flags exist and cause an error on update. This + function removes any migration flags that aren't already present on the target + tenant. See: https://github.com/auth0/auth0-deploy-cli/issues/374 + */ + + const TENANT_MIGRATION_FLAGS = [ + 'disable_clickjack_protection_headers', + 'enable_mgmt_api_v1', + 'trust_azure_adfs_email_verified_connection_property', + 'include_email_in_reset_pwd_redirect', + 'include_email_in_verify_email_redirect', + ]; + + return Object.keys(proposedFlags).reduce( + (acc: Tenant['flags'], proposedKey: string): Tenant['flags'] => { + const isMigrationFlag = TENANT_MIGRATION_FLAGS.includes(proposedKey); + if (!isMigrationFlag) + return { + ...acc, + [proposedKey]: proposedFlags[proposedKey], + }; + + const keyCurrentlyExists = existingFlags[proposedKey] !== undefined; + if (keyCurrentlyExists) + return { + ...acc, + [proposedKey]: proposedFlags[proposedKey], + }; + + return acc; + }, + {} + ); +}; diff --git a/src/types.ts b/src/types.ts index ae987470c..e4b9b334d 100644 --- a/src/types.ts +++ b/src/types.ts @@ -5,7 +5,7 @@ import { PromptsCustomText, PromptSettings, } from './tools/auth0/handlers/prompts'; - +import { Tenant } from './tools/auth0/handlers/tenant'; import { Theme } from './tools/auth0/handlers/themes'; type SharedPaginationParams = { @@ -145,8 +145,8 @@ export type BaseAuth0APIClient = { getAll: () => Promise; }; tenant: APIClientBaseFunctions & { - getSettings: () => Promise; - updateSettings: (arg0: Asset) => Promise; + getSettings: () => Promise; + updateSettings: (arg0: Partial) => Promise; }; triggers: APIClientBaseFunctions & { getTriggerBindings: () => Promise; @@ -230,7 +230,7 @@ export type Assets = Partial<{ roles: Asset[] | null; rules: Asset[] | null; rulesConfigs: Asset[] | null; - tenant: Asset | null; + tenant: Tenant | null; triggers: Asset[] | null; //non-resource types exclude?: { diff --git a/test/tools/auth0/handlers/tenant.tests.js b/test/tools/auth0/handlers/tenant.tests.js index f0eff87cb..f948994d8 100644 --- a/test/tools/auth0/handlers/tenant.tests.js +++ b/test/tools/auth0/handlers/tenant.tests.js @@ -52,4 +52,88 @@ describe('#tenant handler', () => { await stageFn.apply(handler, [{ tenant: { sandbox_version: '4' } }]); }); }); + + describe('#removeUnapplicableMigrationFlags function', () => { + it('should not alter flags if existing and proposed are identical', () => { + const flags = { + trust_azure_adfs_email_verified_connection_property: true, + 'some-flag-1': false, + 'some-flag-2': true, + }; + + const result = tenant.removeUnapplicableMigrationFlags(flags, flags); + + expect(result).to.deep.equal(flags); + }); + + it("should not remove migration flag if proposed but doesn't exist currently", () => { + const existingFlags = { + 'some-flag-1': false, + 'some-flag-2': true, + }; + + const proposedFlags = { + trust_azure_adfs_email_verified_connection_property: true, // Migration flag + 'some-flag-1': true, + 'some-flag-2': true, + }; + + const result = tenant.removeUnapplicableMigrationFlags(existingFlags, proposedFlags); + + const expectedFlags = (() => { + const expected = proposedFlags; + delete expected.trust_azure_adfs_email_verified_connection_property; + return expected; + })(); + + expect(result).to.deep.equal(expectedFlags); + }); + + it('allow alterations to migration flags if they currently exist', () => { + const existingFlags = { + 'some-flag-1': false, + 'some-flag-2': true, + trust_azure_adfs_email_verified_connection_property: false, // Migration flag + }; + + const proposedFlags = { + trust_azure_adfs_email_verified_connection_property: true, // Migration flag + 'some-flag-1': true, + 'some-flag-2': false, + }; + + const result = tenant.removeUnapplicableMigrationFlags(existingFlags, proposedFlags); + + expect(result).to.deep.equal(proposedFlags); + }); + + it('should allow alterations of non-migration flags even if they do not currently exist', () => { + const existingFlags = { + trust_azure_adfs_email_verified_connection_property: true, + }; + + const proposedFlags = { + trust_azure_adfs_email_verified_connection_property: true, + 'some-flag-1': false, // Doesn't currently exist + 'some-flag-2': true, // Doesn't currently exist + 'some-flag-3': true, // Doesn't currently exist + }; + + const result = tenant.removeUnapplicableMigrationFlags(existingFlags, proposedFlags); + + expect(result).to.deep.equal(proposedFlags); + }); + + it('should not throw if allow empty flag objects passed', () => { + const mockFlags = { + trust_azure_adfs_email_verified_connection_property: true, + 'some-flag-1': false, // Doesn't currently exist + 'some-flag-2': true, // Doesn't currently exist + }; + + expect(() => tenant.removeUnapplicableMigrationFlags({}, {})).to.not.throw(); + expect(() => tenant.removeUnapplicableMigrationFlags(mockFlags, {})).to.not.throw(); + expect(() => tenant.removeUnapplicableMigrationFlags({}, mockFlags)).to.not.throw(); + }); + }); }); From 58b6343d55bebf20dd48d1c8f5e41fb6f51d4871 Mon Sep 17 00:00:00 2001 From: Will Vedder Date: Fri, 24 Jun 2022 12:14:05 -0400 Subject: [PATCH 2/5] Making function name clearer --- src/tools/auth0/handlers/tenant.ts | 13 ++++++++----- test/tools/auth0/handlers/tenant.tests.js | 16 ++++++++-------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/tools/auth0/handlers/tenant.ts b/src/tools/auth0/handlers/tenant.ts index 7da54b6d9..e45a1d981 100644 --- a/src/tools/auth0/handlers/tenant.ts +++ b/src/tools/auth0/handlers/tenant.ts @@ -65,17 +65,20 @@ export default class TenantHandler extends DefaultHandler { const existingTenant = this.existing || this.getType(); - const sanitizedFlags = removeUnapplicableMigrationFlags(tenant.flags, existingTenant.flags); + const updatedTenant = { + ...tenant, + flags: sanitizeMigrationFlags(tenant.flags, existingTenant.flags), + }; - if (tenant && Object.keys(tenant).length > 0) { - await this.client.tenant.updateSettings(tenant); + if (updatedTenant && Object.keys(updatedTenant).length > 0) { + await this.client.tenant.updateSettings(updatedTenant); this.updated += 1; - this.didUpdate(tenant); + this.didUpdate(updatedTenant); } } } -export const removeUnapplicableMigrationFlags = ( +export const sanitizeMigrationFlags = ( existingFlags: Tenant['flags'] = {}, proposedFlags: Tenant['flags'] = {} ): Tenant['flags'] => { diff --git a/test/tools/auth0/handlers/tenant.tests.js b/test/tools/auth0/handlers/tenant.tests.js index f948994d8..0dd2d930b 100644 --- a/test/tools/auth0/handlers/tenant.tests.js +++ b/test/tools/auth0/handlers/tenant.tests.js @@ -53,7 +53,7 @@ describe('#tenant handler', () => { }); }); - describe('#removeUnapplicableMigrationFlags function', () => { + describe('#sanitizeMigrationFlags function', () => { it('should not alter flags if existing and proposed are identical', () => { const flags = { trust_azure_adfs_email_verified_connection_property: true, @@ -61,7 +61,7 @@ describe('#tenant handler', () => { 'some-flag-2': true, }; - const result = tenant.removeUnapplicableMigrationFlags(flags, flags); + const result = tenant.sanitizeMigrationFlags(flags, flags); expect(result).to.deep.equal(flags); }); @@ -78,7 +78,7 @@ describe('#tenant handler', () => { 'some-flag-2': true, }; - const result = tenant.removeUnapplicableMigrationFlags(existingFlags, proposedFlags); + const result = tenant.sanitizeMigrationFlags(existingFlags, proposedFlags); const expectedFlags = (() => { const expected = proposedFlags; @@ -102,7 +102,7 @@ describe('#tenant handler', () => { 'some-flag-2': false, }; - const result = tenant.removeUnapplicableMigrationFlags(existingFlags, proposedFlags); + const result = tenant.sanitizeMigrationFlags(existingFlags, proposedFlags); expect(result).to.deep.equal(proposedFlags); }); @@ -119,7 +119,7 @@ describe('#tenant handler', () => { 'some-flag-3': true, // Doesn't currently exist }; - const result = tenant.removeUnapplicableMigrationFlags(existingFlags, proposedFlags); + const result = tenant.sanitizeMigrationFlags(existingFlags, proposedFlags); expect(result).to.deep.equal(proposedFlags); }); @@ -131,9 +131,9 @@ describe('#tenant handler', () => { 'some-flag-2': true, // Doesn't currently exist }; - expect(() => tenant.removeUnapplicableMigrationFlags({}, {})).to.not.throw(); - expect(() => tenant.removeUnapplicableMigrationFlags(mockFlags, {})).to.not.throw(); - expect(() => tenant.removeUnapplicableMigrationFlags({}, mockFlags)).to.not.throw(); + expect(() => tenant.sanitizeMigrationFlags({}, {})).to.not.throw(); + expect(() => tenant.sanitizeMigrationFlags(mockFlags, {})).to.not.throw(); + expect(() => tenant.sanitizeMigrationFlags({}, mockFlags)).to.not.throw(); }); }); }); From b5d4a620a514675635aef7dd3719bf89c566a4b6 Mon Sep 17 00:00:00 2001 From: Will Vedder Date: Fri, 24 Jun 2022 13:01:53 -0400 Subject: [PATCH 3/5] Formatting --- src/tools/auth0/handlers/tenant.ts | 2 +- test/tools/auth0/handlers/tenant.tests.js | 38 +++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/tools/auth0/handlers/tenant.ts b/src/tools/auth0/handlers/tenant.ts index e45a1d981..244fb4f97 100644 --- a/src/tools/auth0/handlers/tenant.ts +++ b/src/tools/auth0/handlers/tenant.ts @@ -63,7 +63,7 @@ export default class TenantHandler extends DefaultHandler { // Do nothing if not set if (!tenant) return; - const existingTenant = this.existing || this.getType(); + const existingTenant = this.existing || (await this.getType()); const updatedTenant = { ...tenant, diff --git a/test/tools/auth0/handlers/tenant.tests.js b/test/tools/auth0/handlers/tenant.tests.js index 0dd2d930b..3e6cd0ee0 100644 --- a/test/tools/auth0/handlers/tenant.tests.js +++ b/test/tools/auth0/handlers/tenant.tests.js @@ -51,6 +51,44 @@ describe('#tenant handler', () => { await stageFn.apply(handler, [{ tenant: { sandbox_version: '4' } }]); }); + + it("should remove migration flags if don't exist on target tenant", async () => { + const mockFlags = { + trust_azure_adfs_email_verified_connection_property: true, // Migration flag + 'some-flag-1': true, + 'some-flag-2': false, + }; + + const auth0 = { + tenant: { + getSettings: async () => { + const flags = { ...mockFlags }; + delete flags.trust_azure_adfs_email_verified_connection_property; + return Promise.resolve({ + friendly_name: 'Test', + default_directory: 'users', + flags, + }); + }, + updateSettings: (data) => { + expect(data).to.be.an('object'); + expect(data.flags).to.deep.equal( + (() => { + const flags = { ...mockFlags }; + delete flags.trust_azure_adfs_email_verified_connection_property; + return flags; + })() + ); + return Promise.resolve(data); + }, + }, + }; + + const handler = new tenant.default({ client: auth0 }); + const { processChanges } = Object.getPrototypeOf(handler); + + await processChanges.apply(handler, [{ tenant: { flags: mockFlags } }]); + }); }); describe('#sanitizeMigrationFlags function', () => { From 54c8eb1d7239ae84d7fedeb9fba3286bece5c101 Mon Sep 17 00:00:00 2001 From: Will Vedder Date: Fri, 24 Jun 2022 13:05:18 -0400 Subject: [PATCH 4/5] Adding getSettings to tenant integration tests --- src/tools/auth0/handlers/tenant.ts | 4 ++-- test/tools/auth0/handlers/tenant.tests.js | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/tools/auth0/handlers/tenant.ts b/src/tools/auth0/handlers/tenant.ts index 244fb4f97..0ac232441 100644 --- a/src/tools/auth0/handlers/tenant.ts +++ b/src/tools/auth0/handlers/tenant.ts @@ -90,7 +90,7 @@ export const sanitizeMigrationFlags = ( tenant. See: https://github.com/auth0/auth0-deploy-cli/issues/374 */ - const TENANT_MIGRATION_FLAGS = [ + const tenantMigrationFlags = [ 'disable_clickjack_protection_headers', 'enable_mgmt_api_v1', 'trust_azure_adfs_email_verified_connection_property', @@ -100,7 +100,7 @@ export const sanitizeMigrationFlags = ( return Object.keys(proposedFlags).reduce( (acc: Tenant['flags'], proposedKey: string): Tenant['flags'] => { - const isMigrationFlag = TENANT_MIGRATION_FLAGS.includes(proposedKey); + const isMigrationFlag = tenantMigrationFlags.includes(proposedKey); if (!isMigrationFlag) return { ...acc, diff --git a/test/tools/auth0/handlers/tenant.tests.js b/test/tools/auth0/handlers/tenant.tests.js index 3e6cd0ee0..510946ff8 100644 --- a/test/tools/auth0/handlers/tenant.tests.js +++ b/test/tools/auth0/handlers/tenant.tests.js @@ -38,6 +38,10 @@ describe('#tenant handler', () => { it('should update tenant settings', async () => { const auth0 = { tenant: { + getSettings: () => ({ + friendly_name: 'Test', + default_directory: 'users', + }), updateSettings: (data) => { expect(data).to.be.an('object'); expect(data.sandbox_version).to.equal('4'); From e187cb5b8cf0c2b8a74269aa67a2c21deb6c1323 Mon Sep 17 00:00:00 2001 From: Will Vedder Date: Fri, 24 Jun 2022 13:21:17 -0400 Subject: [PATCH 5/5] Refactor to prevent confusino of order of arguments --- src/tools/auth0/handlers/tenant.ts | 16 +++++++++----- test/tools/auth0/handlers/tenant.tests.js | 26 +++++++++++++++++------ 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/tools/auth0/handlers/tenant.ts b/src/tools/auth0/handlers/tenant.ts index 0ac232441..a384a31c6 100644 --- a/src/tools/auth0/handlers/tenant.ts +++ b/src/tools/auth0/handlers/tenant.ts @@ -67,7 +67,10 @@ export default class TenantHandler extends DefaultHandler { const updatedTenant = { ...tenant, - flags: sanitizeMigrationFlags(tenant.flags, existingTenant.flags), + flags: sanitizeMigrationFlags({ + existingFlags: existingTenant.flags, + proposedFlags: tenant.flags, + }), }; if (updatedTenant && Object.keys(updatedTenant).length > 0) { @@ -78,10 +81,13 @@ export default class TenantHandler extends DefaultHandler { } } -export const sanitizeMigrationFlags = ( - existingFlags: Tenant['flags'] = {}, - proposedFlags: Tenant['flags'] = {} -): Tenant['flags'] => { +export const sanitizeMigrationFlags = ({ + existingFlags = {}, + proposedFlags = {}, +}: { + existingFlags: Tenant['flags']; + proposedFlags: Tenant['flags']; +}): Tenant['flags'] => { /* Tenants can only update migration flags that are already configured. If moving configuration from one tenant to another, there may be instances diff --git a/test/tools/auth0/handlers/tenant.tests.js b/test/tools/auth0/handlers/tenant.tests.js index 510946ff8..530ccffcf 100644 --- a/test/tools/auth0/handlers/tenant.tests.js +++ b/test/tools/auth0/handlers/tenant.tests.js @@ -103,7 +103,10 @@ describe('#tenant handler', () => { 'some-flag-2': true, }; - const result = tenant.sanitizeMigrationFlags(flags, flags); + const result = tenant.sanitizeMigrationFlags({ + existingFlags: flags, + proposedFlags: flags, + }); expect(result).to.deep.equal(flags); }); @@ -120,7 +123,10 @@ describe('#tenant handler', () => { 'some-flag-2': true, }; - const result = tenant.sanitizeMigrationFlags(existingFlags, proposedFlags); + const result = tenant.sanitizeMigrationFlags({ + existingFlags, + proposedFlags, + }); const expectedFlags = (() => { const expected = proposedFlags; @@ -144,7 +150,7 @@ describe('#tenant handler', () => { 'some-flag-2': false, }; - const result = tenant.sanitizeMigrationFlags(existingFlags, proposedFlags); + const result = tenant.sanitizeMigrationFlags({ existingFlags, proposedFlags }); expect(result).to.deep.equal(proposedFlags); }); @@ -161,7 +167,7 @@ describe('#tenant handler', () => { 'some-flag-3': true, // Doesn't currently exist }; - const result = tenant.sanitizeMigrationFlags(existingFlags, proposedFlags); + const result = tenant.sanitizeMigrationFlags({ existingFlags, proposedFlags }); expect(result).to.deep.equal(proposedFlags); }); @@ -173,9 +179,15 @@ describe('#tenant handler', () => { 'some-flag-2': true, // Doesn't currently exist }; - expect(() => tenant.sanitizeMigrationFlags({}, {})).to.not.throw(); - expect(() => tenant.sanitizeMigrationFlags(mockFlags, {})).to.not.throw(); - expect(() => tenant.sanitizeMigrationFlags({}, mockFlags)).to.not.throw(); + expect(() => + tenant.sanitizeMigrationFlags({ existingFlags: {}, proposedFlags: {} }) + ).to.not.throw(); + expect(() => + tenant.sanitizeMigrationFlags({ existingFlags: mockFlags, proposedFlags: {} }) + ).to.not.throw(); + expect(() => + tenant.sanitizeMigrationFlags({ existingFlags: {}, proposedFlags: mockFlags }) + ).to.not.throw(); }); }); });