Skip to content

Commit

Permalink
feat(login): let users opt-out of sync when signing in to Firefox
Browse files Browse the repository at this point in the history
Fixes #2396
  • Loading branch information
vladikoff committed Sep 30, 2019
1 parent 22e4e7b commit c9f6a95
Show file tree
Hide file tree
Showing 33 changed files with 696 additions and 47 deletions.
10 changes: 7 additions & 3 deletions packages/fxa-content-server/app/scripts/lib/app-start.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import Session from './session';
import Storage from './storage';
import StorageMetrics from './storage-metrics';
import SupplicantRelier from '../models/reliers/pairing/supplicant';
import SyncRelier from '../models/reliers/sync';
import BrowserRelier from '../models/reliers/browser';
import Translator from './translator';
import UniqueUserId from '../models/unique-user-id';
import Url from './url';
Expand Down Expand Up @@ -269,8 +269,12 @@ Start.prototype = {
window: this._window,
}
);
} else if (this._isServiceSync()) {
relier = new SyncRelier(
} else if (
this._isServiceSync() ||
// context v3 is able to sign in to Firefox without enabling Sync
this._searchParam('context') === Constants.FX_DESKTOP_V3_CONTEXT
) {
relier = new BrowserRelier(
{ context },
{
isVerification: this._isVerification(),
Expand Down
2 changes: 2 additions & 0 deletions packages/fxa-content-server/app/scripts/lib/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import SubscriptionsProductRedirectView from '../views/subscriptions_product_red
import SubscriptionsManagementRedirectView from '../views/subscriptions_management_redirect';
import TwoStepAuthenticationView from '../views/settings/two_step_authentication';
import VerificationReasons from './verification-reasons';
import WouldYouLikeToSync from '../views/would_you_like_to_sync';
import WhyConnectAnotherDeviceView from '../views/why_connect_another_device';

function getView(ViewOrPath) {
Expand Down Expand Up @@ -278,6 +279,7 @@ const Router = Backbone.Router.extend({
'verify_secondary_email(/)': createViewHandler(CompleteSignUpView, {
type: VerificationReasons.SECONDARY_EMAIL_VERIFIED,
}),
'would_you_like_to_sync(/)': createViewHandler(WouldYouLikeToSync),
},

initialize(options = {}) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const ALLOWED_LOGIN_FIELDS = [
'keyFetchToken',
'offeredSyncEngines',
'sessionToken',
'services',
'uid',
'unwrapBKey',
'verified',
Expand Down Expand Up @@ -273,11 +274,39 @@ const FxSyncChannelAuthenticationBroker = FxSyncAuthenticationBroker.extend(
* @private
*/
_getLoginData(account) {
const loginData = account.pick(ALLOWED_LOGIN_FIELDS);
let loginData = account.pick(ALLOWED_LOGIN_FIELDS) || {};
const isMultiService = this.relier && this.relier.get('multiService');
if (isMultiService) {
loginData = this._formatForMultiServiceBrowser(loginData);
}

loginData.verified = !!loginData.verified;
loginData.verifiedCanLinkAccount = !!this._verifiedCanLinkEmail;
return _.omit(loginData, _.isUndefined);
},

/**
* if browser is multi service capable then we should send 'sync' properties
* in a different format
* @param loginData
* @private
*/
_formatForMultiServiceBrowser(loginData) {
loginData.services = {};
if (!this.relier.get('doNotSync') && loginData.offeredSyncEngines) {
// make sure to NOT send an empty 'sync' object,
// this can happen in the `force_auth` flow.
loginData.services.sync = {
offeredEngines: loginData.offeredSyncEngines,
declinedEngines: loginData.declinedSyncEngines,
};
}
// these should not be sent to a multi-service capable browser
delete loginData.offeredSyncEngines;
delete loginData.declinedSyncEngines;

return loginData;
},
},
{
REQUIRED_LOGIN_FIELDS,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,19 @@ export default BaseAuthenticationBroker.extend({
*/
onFxaStatus(response = {}) {
const syncEngines = this.get('chooseWhatToSyncWebV1Engines');
const multiService =
response.capabilities && response.capabilities.multiService;
this.relier.set('multiService', multiService);
if (multiService) {
// we get the OAuth client id for the browser
// in order to replicate the uses of the 'service' param on the backend.
// See: https://github.com/mozilla/fxa/issues/2396#issuecomment-530662772
if (!this.relier.has('service')) {
// the service in the query parameter currently overrides the status message
// this is due to backwards compatibility
this.relier.set('service', response.clientId);
}
}
const additionalEngineIds =
response.capabilities && response.capabilities.engines;
if (syncEngines && additionalEngineIds) {
Expand Down
10 changes: 10 additions & 0 deletions packages/fxa-content-server/app/scripts/models/reliers/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import Backbone from 'backbone';
var Relier = Backbone.Model.extend({
defaults: {
context: null,
name: 'base',
},

fetch() {
Expand All @@ -36,6 +37,15 @@ var Relier = Backbone.Model.extend({
return false;
},

/**
* Check if the relier should offer Sync
*
* @returns {Boolean}
*/
shouldOfferToSync() {
return false;
},

/**
* Check if the relier wants access to the account encryption keys.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ const QUERY_PARAMETER_SCHEMA = {
export default Relier.extend({
defaults: _.extend({}, Relier.prototype.defaults, {
action: undefined,
name: 'browser',
doNotSync: false,
multiService: false,
signinCode: undefined,
tokenCode: false,
}),
Expand All @@ -50,6 +53,9 @@ export default Relier.extend({

if (this.get('service')) {
this.set('serviceName', t('Firefox Sync'));
} else {
// if no service provided, then we are just signing into the browser
this.set('serviceName', t('Firefox'));
}
});
},
Expand All @@ -58,6 +64,10 @@ export default Relier.extend({
return true;
},

shouldOfferToSync(viewName) {
return this.get('service') !== 'sync' && viewName !== 'force-auth';
},

/**
* Desktop clients will always want keys so they can sync.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ var OAuthRelier = Relier.extend({
acrValues: null,
clientId: null,
context: Constants.OAUTH_CONTEXT,
name: 'oauth',
keysJwk: null,
// permissions are individual scopes
permissions: null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ const AUTHORITY_QUERY_PARAM_SCHEMA = {
/*eslint-enable camelcase, sorting/sort-object-props*/

export default class AuthorityRelier extends OAuthRelier {
name = 'pairing-authority';

fetch() {
return Promise.resolve().then(() => {
this.importSearchParamsUsingSchema(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ const SUPPLICANT_HASH_PARAMETER_SCHEMA = {
/*eslint-enable camelcase, sorting/sort-object-props*/

export default class SupplicantRelier extends OAuthRelier {
name = 'pairing-supplicant';

fetch() {
return Promise.resolve().then(() => {
this.importHashParamsUsingSchema(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
<div id="main-content" class="card choose-what-to-sync">
<div class="success visible success-email-created">{{#t}}%(email)s registered{{/t}}</div>
<div class="error"></div>
<div class="graphic graphic-choose-what-to-sync"></div>

Expand All @@ -17,9 +16,14 @@
</div>
{{/engines}}
</div>
<div>
<button id="submit-btn" type="submit" tabindex="1" autofocus>{{#t}}Continue{{/t}}</button>
<div class="button-row">
<button id="submit-btn" type="submit" tabindex="1" autofocus>{{#t}}Enable Sync{{/t}}</button>
</div>
{{#showDoNotSyncButton}}
<div class="links centered">
<a id="do-not-sync-device" href="#" class="button-link">{{#t}}Don’t sync this device{{/t}}</a>
</div>
{{/showDoNotSyncButton}}
</form>
</section>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<div id="main-content" class="card would-you-like-to-sync">
<div class="error"></div>
<div class="graphic graphic-choose-what-to-sync"></div>

<section>
<form id="would-you-like-to-sync" novalidate>
<header>
<h1 id="fxa-would-you-like-to-sync-header">
{{#t}}Would you like to sync this device to your account?{{/t}}
</h1>
</header>
<div class="button-row">
<button id="submit-btn" type="submit" tabindex="1" autofocus>{{#t}}Sync this device{{/t}}</button>
</div>
<div class="links centered">
<a id="do-not-sync-device" href="#" class="button-link">{{#t}}Don’t sync this device{{/t}}</a>
</div>
</form>
</section>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import _ from 'underscore';
import $ from 'jquery';
import BackMixin from './mixins/back-mixin';
import Cocktail from 'cocktail';
import DoNotSyncMixin from './mixins/do-not-sync-mixin';
import FlowEventsMixin from './mixins/flow-events-mixin';
import FormView from './form';
import SessionVerificationPollMixin from './mixins/session-verification-poll-mixin';
Expand All @@ -19,14 +20,21 @@ const View = FormView.extend(
template: Template,
className: 'choose-what-to-sync',

initialize(options = {}) {
initialize() {
// Account data is passed in from sign up flow.
this._account = this.user.initAccount(this.model.get('account'));

// to keep the view from knowing too much about the state machine,
// a continuation function is passed in that should be called
// when submit has completed.
this.onSubmitComplete = this.model.get('onSubmitComplete');

// we only want to show the do not sync button for multi-service browser,
// that are not logging into sync and that came from a sign up flow.
this.allowToDisableSync = this.model.get('allowToDisableSync');
// in some cases where the user has a choice to decline Sync
// we don't want to poll for verification
this.pollVerification = this.model.get('pollVerification');
},

getAccount() {
Expand Down Expand Up @@ -59,9 +67,13 @@ const View = FormView.extend(
// See #5554
.then(() => this.broker.persistVerificationData(account))
.then(() => {
this.waitForSessionVerification(account, () =>
this.validateAndSubmit()
);
if (this.pollVerification) {
// we should only wait for session verification here
// if the user doesn't have an option to disable sync
this.waitForSessionVerification(account, () =>
this.validateAndSubmit()
);
}
})
);
},
Expand All @@ -72,12 +84,13 @@ const View = FormView.extend(
},

setInitialContext(context) {
var account = this.getAccount();
const account = this.getAccount();
const engines = this._getOfferedEngines();

context.set({
email: account.get('email'),
engines,
showDoNotSyncButton: this.allowToDisableSync,
});
},

Expand Down Expand Up @@ -166,6 +179,12 @@ const View = FormView.extend(
}
);

Cocktail.mixin(View, BackMixin, FlowEventsMixin, SessionVerificationPollMixin);
Cocktail.mixin(
View,
BackMixin,
FlowEventsMixin,
SessionVerificationPollMixin,
DoNotSyncMixin
);

export default View;
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/* 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/. */

/**
* View mixin to handle the Do Not Sync button in views.
*
* @mixin DoNotSync mixin
*/
import SigninMixin from './signin-mixin';

export default {
dependsOn: [SigninMixin],

events: {
'click #do-not-sync-device': 'doNotSync',
},

doNotSync() {
this.relier.set('doNotSync', true);
return Promise.resolve().then(() => {
const account = this.getAccount();
return this.onSubmitComplete(account);
});
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,18 @@ export default {
});
}

if (this.relier.shouldOfferToSync(this.viewName)) {
// flows that are a part of the 'browser' relier which
// do not pass a service get asked of they want to Sync

// force_auth attempts do not a choice for Sync
return this.navigate('would_you_like_to_sync', {
account: account,
// propagate the onSubmitComplete to choose_what_to_sync screen if needed
onSubmitComplete: this.onSignInSuccess.bind(this),
});
}

if (typeof options.onSuccess === 'function') {
options.onSuccess();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ export default {
} else if (this.broker.get('chooseWhatToSyncWebV1Engines')) {
return this.navigate('choose_what_to_sync', {
account: account,
allowToDisableSync: this.relier.get('service') !== 'sync',
pollVerification: this.relier.get('service') === 'sync',
// choose_what_to_sync screen will call onSubmitComplete
// with an updated account
onSubmitComplete: onSubmitComplete,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ export default function(config) {
* @returns {Boolean}
*/
isSyncSuggestionEnabled() {
return !this.relier.get('service');
return (
!this.relier.get('service') && this.relier.get('context') === 'web'
);
},

onSuggestSyncDismiss() {
Expand Down
Loading

0 comments on commit c9f6a95

Please sign in to comment.