Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

Commit

Permalink
feat(pairing): disable pairing for 2FA accounts
Browse files Browse the repository at this point in the history
Fixes #6987
  • Loading branch information
vladikoff committed Mar 27, 2019
1 parent 2dc7794 commit 7fd394e
Show file tree
Hide file tree
Showing 8 changed files with 91 additions and 4 deletions.
6 changes: 5 additions & 1 deletion app/scripts/lib/auth-errors.js
Expand Up @@ -526,7 +526,11 @@ define(function (require, exports, module) {
TOKEN_VERIFICATION_CODE_REQUIRED: {
errno: 1060,
message: t('Please enter verification code')
}
},
TOTP_PAIRING_NOT_SUPPORTED: {
errno: 1061,
message: t('Accounts with two-step authentication do not support pairing at this time')
},
};
/*eslint-enable sorting/sort-object-props*/

Expand Down
6 changes: 5 additions & 1 deletion app/scripts/views/pair/auth_allow.js
Expand Up @@ -5,6 +5,7 @@
import { assign } from 'underscore';
import Cocktail from 'cocktail';
import DeviceBeingPairedMixin from './device-being-paired-mixin';
import PairingTotpMixin from './pairing-totp-mixin';
import FormView from '../form';
import { preventDefaultThen } from '../base';
import Template from '../../templates/pair/auth_allow.mustache';
Expand All @@ -16,8 +17,10 @@ class PairAuthAllowView extends FormView {
'click #cancel': preventDefaultThen('cancel')
});

initialize () {
beforeRender () {
this.listenTo(this.broker, 'error', this.displayError);

return this.checkTotpStatus();
}

submit () {
Expand All @@ -32,6 +35,7 @@ class PairAuthAllowView extends FormView {

Cocktail.mixin(
PairAuthAllowView,
PairingTotpMixin(),
DeviceBeingPairedMixin(),
);

Expand Down
6 changes: 5 additions & 1 deletion app/scripts/views/pair/index.js
Expand Up @@ -7,6 +7,7 @@ import Cocktail from 'cocktail';
import Template from '../../templates/pair/index.mustache';
import UserAgentMixin from '../../lib/user-agent-mixin';
import PairingGraphicsMixin from '../mixins/pairing-graphics-mixin';
import PairingTotpMixin from './pairing-totp-mixin';
import { DOWNLOAD_LINK_PAIRING_APP } from '../../lib/constants';

class PairIndexView extends FormView {
Expand Down Expand Up @@ -34,6 +35,8 @@ class PairIndexView extends FormView {
if (! this.broker.hasCapability('supportsPairing')) {
return this.replaceCurrentPage('pair/unsupported');
}

return this.checkTotpStatus();
}

setInitialContext (context) {
Expand All @@ -50,7 +53,8 @@ class PairIndexView extends FormView {
Cocktail.mixin(
PairIndexView,
PairingGraphicsMixin,
UserAgentMixin
PairingTotpMixin(),
UserAgentMixin,
);

export default PairIndexView;
28 changes: 28 additions & 0 deletions app/scripts/views/pair/pairing-totp-mixin.js
@@ -0,0 +1,28 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import AuthErrors from '../../lib/auth-errors';

export default function () {
return {
checkTotpStatus() {
const account = this.getSignedInAccount();

if (! account) {
this.replaceCurrentPage('pair/failure', {
error: AuthErrors.toError('UNKNOWN_ACCOUNT'),
});
}

return account.checkTotpTokenExists().then((result) => {
// pairing is disabled for accounts with 2FA
if (result.exists) {
this.replaceCurrentPage('pair/failure', {
error: AuthErrors.toError('TOTP_PAIRING_NOT_SUPPORTED'),
});
}
});
}
};
}
20 changes: 20 additions & 0 deletions app/tests/spec/views/pair/auth_allow.js
Expand Up @@ -4,6 +4,7 @@

import $ from 'jquery';
import { assert } from 'chai';
import AuthErrors from 'lib/auth-errors';
import AuthorityBroker from 'models/auth_brokers/pairing/authority';
import Notifier from 'lib/channels/notifier';
import Relier from 'models/reliers/relier';
Expand All @@ -24,6 +25,7 @@ const REMOTE_METADATA = {
};

describe('views/pair/auth_allow', () => {
let account;
let broker;
let config;
let relier;
Expand All @@ -41,6 +43,10 @@ describe('views/pair/auth_allow', () => {
clientId: '3c49430b43dfba77',
});
user = new User();
account = user.initAccount();
sinon.stub(account, 'checkTotpTokenExists').callsFake(() => {
return Promise.resolve({exists: false});
});
notifier = new Notifier();
broker = new AuthorityBroker({
config,
Expand All @@ -66,6 +72,7 @@ describe('views/pair/auth_allow', () => {
viewName: 'pairAuthAllow',
window: windowMock
});
sinon.stub(view, 'getSignedInAccount').callsFake(() => account);
}

describe('render', () => {
Expand Down Expand Up @@ -98,5 +105,18 @@ describe('views/pair/auth_allow', () => {
}, 1);
});
});

it('blocks users with TOTP', () => {
account.checkTotpTokenExists.restore();
sinon.stub(account, 'checkTotpTokenExists').callsFake(() => {
return Promise.resolve({exists: true});
});
sinon.spy(view, 'replaceCurrentPage');
return view.render()
.then(() => {
assert.isTrue(view.replaceCurrentPage.calledOnceWith('pair/failure'));
assert.equal(view.replaceCurrentPage.args[0][1].error.message, AuthErrors.toMessage(1061));
});
});
});
});
9 changes: 9 additions & 0 deletions app/tests/spec/views/pair/index.js
Expand Up @@ -7,12 +7,15 @@ import sinon from 'sinon';
import View from 'views/pair/index';
import BaseBroker from 'models/auth_brokers/base';
import Relier from 'models/reliers/relier';
import User from 'models/user';
import WindowMock from '../../../mocks/window';

const UA_CHROME = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3626.109 Safari/537.36';
const UA_FIREFOX = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:67.0) Gecko/20100101 Firefox/67.0';

describe('views/pair/index', () => {
let account;
let user;
let view;
let broker;
let relier;
Expand All @@ -23,6 +26,11 @@ describe('views/pair/index', () => {
relier = new Relier({}, {
window: windowMock
});
user = new User();
account = user.initAccount();
sinon.stub(account, 'checkTotpTokenExists').callsFake(() => {
return Promise.resolve({exists: false});
});
broker = new BaseBroker({
relier: relier,
window: windowMock
Expand All @@ -34,6 +42,7 @@ describe('views/pair/index', () => {
});
sinon.stub(view, 'navigate');
sinon.spy(view, 'replaceCurrentPage');
sinon.stub(view, 'getSignedInAccount').callsFake(() => account);
});

afterEach(function () {
Expand Down
1 change: 1 addition & 0 deletions tests/functional/lib/selectors.js
Expand Up @@ -146,6 +146,7 @@ module.exports = {
PAIRING: {
AUTH_SUBMIT: '#auth-approve-btn',
COMPLETE: '#fxa-oauth-success-header',
PAIR_FAILURE: '#fxa-pair-failure-header',
START_PAIRING: '#start-pairing',
SUPP_SUBMIT: '#supp-approve-btn',
},
Expand Down
19 changes: 18 additions & 1 deletion tests/functional/pairing.js
Expand Up @@ -19,6 +19,7 @@ const BAD_OAUTH_REDIRECT = `${config.fxaOAuthApp}api/oauth`;
const GOOD_CLIENT_ID = '3c49430b43dfba77';
const GOOD_PAIR_URL = `${config.fxaContentRoot}pair/supp?response_type=code&client_id=${GOOD_CLIENT_ID}&redirect_uri=${REDIRECT_HOST}oauth%2Fsuccess%2F3c49430b43dfba77&scope=profile%2Bhttps%3A%2F%2Fidentity.mozilla.com%2Fapps%2Foldsync&state=foo&code_challenge_method=S256&code_challenge=IpOAcntLUmKITcxI_rDqMvFTeC9n_g0B8_Pj2yWZp7w&access_type=offline&keys_jwk=eyJjcnYiOiJQLTI1NiIsImt0eSI6IkVDIiwieCI6ImlmcWY2U1pwMlM0ZjA5c3VhS093dmNsbWJxUm8zZXdGY0pvRURpYnc4MTQiLCJ5IjoiSE9LTXh5c1FseExqRGttUjZZbFpaY1Y4MFZBdk9nSWo1ZHRVaWJmYy1qTSJ9`; //eslint-disable-line max-len
const BAD_PAIR_URL = `${config.fxaContentRoot}pair/supp?response_type=code&client_id=${BAD_CLIENT_ID}&redirect_uri=${BAD_OAUTH_REDIRECT}&scope=profile%2Bhttps%3A%2F%2Fidentity.mozilla.com%2Fapps%2Foldsync&state=foo&code_challenge_method=S256&code_challenge=IpOAcntLUmKITcxI_rDqMvFTeC9n_g0B8_Pj2yWZp7w&access_type=offline&keys_jwk=eyJjcnYiOiJQLTI1NiIsImt0eSI6IkVDIiwieCI6ImlmcWY2U1pwMlM0ZjA5c3VhS093dmNsbWJxUm8zZXdGY0pvRURpYnc4MTQiLCJ5IjoiSE9LTXh5c1FseExqRGttUjZZbFpaY1Y4MFZBdk9nSWo1ZHRVaWJmYy1qTSJ9`; //eslint-disable-line max-len
const SETTINGS_URL = `${config.fxaContentRoot}settings`;

const PASSWORD = 'PASSWORD123123';
let email;
Expand All @@ -27,6 +28,7 @@ const {
createUser,
click,
closeCurrentWindow,
confirmTotpCode,
openPage,
openTab,
switchToWindow,
Expand Down Expand Up @@ -137,7 +139,22 @@ registerSuite('pairing', {
assert.ok(redirectResult.includes('state='), 'final OAuth redirect has the state');
})
.end()
.then(closeCurrentWindow());
.then(closeCurrentWindow())

// enable 2FA and attempt to pair again, will result in error
.then(openPage(SETTINGS_URL, selectors.TOTP.MENU_BUTTON))

.then(click(selectors.TOTP.MENU_BUTTON))
.then(click(selectors.TOTP.SHOW_CODE_LINK))

.findByCssSelector(selectors.TOTP.MANUAL_CODE)
.getVisibleText()
.then((secretKey) => {
return this.remote.then(confirmTotpCode(secretKey));
})
.end()

.then(openPage(`${config.fxaContentRoot}pair`, selectors.PAIRING.PAIR_FAILURE));
},

'handles invalid clients': function () {
Expand Down

0 comments on commit 7fd394e

Please sign in to comment.