Skip to content

Commit

Permalink
fix(content-server): Fix issue on login with legacy accounts
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dschom committed Apr 26, 2022
1 parent 3a1c36f commit b301bc1
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 1 deletion.
13 changes: 12 additions & 1 deletion packages/fxa-content-server/app/scripts/models/account.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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');
}
Expand Down
21 changes: 21 additions & 0 deletions packages/fxa-content-server/app/tests/spec/models/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down

0 comments on commit b301bc1

Please sign in to comment.