From b301bc11a096c299e4d21dbb1705ad76660f4e89 Mon Sep 17 00:00:00 2001 From: dschom Date: Tue, 26 Apr 2022 14:37:35 -0700 Subject: [PATCH] fix(content-server): Fix issue on login with legacy accounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Because: - There was a regression in the last maintenance sprint when removing the ecosystemAnonId. This Commit: - Adds a collection of ​​DEPRECATED_KEYS that represent keys which may exist on legacy account data and need to be gracefully removed. - Adds a test to make sure that a known deprecated field is handled and does not result in an error state. - Adds a test to make sure that errors are still thrown for invalid keys. --- .../app/scripts/models/account.js | 13 +++++++++++- .../app/tests/spec/models/user.js | 21 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/fxa-content-server/app/scripts/models/account.js b/packages/fxa-content-server/app/scripts/models/account.js index d01dea4642d..1013c83a8ce 100644 --- a/packages/fxa-content-server/app/scripts/models/account.js +++ b/packages/fxa-content-server/app/scripts/models/account.js @@ -80,6 +80,7 @@ const DEFAULTS = _.extend( const ALLOWED_KEYS = Object.keys(DEFAULTS); const ALLOWED_PERSISTENT_KEYS = Object.keys(PERSISTENT); +const DEPRECATED_KEYS = ['ecosystemAnonId']; const CONTENT_SERVER_OAUTH_SCOPE = 'profile profile:write clients:write'; @@ -651,7 +652,7 @@ const Account = Backbone.Model.extend( ) { updatedSessionData.email = options.originalLoginEmail; } - + // We don't really need this value other than in login flow, it can // sometimes cause issues when user switches primary email this.unset('originalLoginEmail'); @@ -1022,6 +1023,16 @@ const Account = Backbone.Model.extend( // eslint-disable-next-line no-unused-vars for (const key in attributes) { + // As fields are phased out and no longer needed, they may drop out + // of the set of ALLOWED_KEYS. In this case, we should no longer store + // or use the associated key; however, it may still exist in a client's + // cached state (e.g. in local storage). The following ensures that + // deprecated keys are cleaned up over time. + if (_.contains(DEPRECATED_KEYS, key)) { + delete attributes[key]; + continue; + } + if (!_.contains(ALLOWED_KEYS, key)) { throw new Error(key + ' cannot be set on an Account'); } diff --git a/packages/fxa-content-server/app/tests/spec/models/user.js b/packages/fxa-content-server/app/tests/spec/models/user.js index f7812691e91..c1c64fdb955 100644 --- a/packages/fxa-content-server/app/tests/spec/models/user.js +++ b/packages/fxa-content-server/app/tests/spec/models/user.js @@ -126,6 +126,27 @@ describe('models/user', function () { }); }); + it('will not create invalid account', function () { + assert.throws(() => { + user.initAccount({ + email: EMAIL, + boom: 'yes', + }); + }, 'boom cannot be set on an Account'); + }); + + it('will handles deprecated settings account', function () { + const account = user.initAccount({ + email: EMAIL, + ecosystemAnonId: '123', + }); + + assert.equal(account.get('email'), EMAIL); + assert.deepEqual(account.pick('email'), { email: EMAIL }); + assert.equal(account._notifier, notifier); + assert.notExists(account.ecosystemAnonId); + }); + describe('sessionStatus', () => { it('checks passed in account', () => { const account = user.initAccount({