Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(content-server): Fix issue on login with legacy accounts #12663

Merged
merged 1 commit into from
Apr 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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