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

Commit

Permalink
Merge pull request #1725 from mozilla/issue-1723-oauth-completion-imp…
Browse files Browse the repository at this point in the history
…rovements

feat(client): Smooth out the verification flow.
  • Loading branch information
zaach committed Oct 7, 2014
2 parents d1a3a78 + 02b0d35 commit 8ba4336
Show file tree
Hide file tree
Showing 25 changed files with 1,624 additions and 441 deletions.
18 changes: 14 additions & 4 deletions app/scripts/lib/app-start.js
Expand Up @@ -35,6 +35,7 @@ define([
'lib/constants',
'lib/oauth-client',
'lib/auth-errors',
'lib/channels/inter-tab',
'models/reliers/relier',
'models/reliers/oauth',
'models/reliers/fx-desktop'
Expand All @@ -57,6 +58,7 @@ function (
Constants,
OAuthClient,
AuthErrors,
InterTabChannel,
Relier,
OAuthRelier,
FxDesktopRelier
Expand Down Expand Up @@ -91,7 +93,8 @@ function (
// fetch both config and translations in parallel to speed up load.
return p.all([
this.initializeConfig(),
this.initializeL10n()
this.initializeL10n(),
this.initializeInterTabChannel()
])
.then(_.bind(this.allResourcesReady, this))
.then(null, function (err) {
Expand All @@ -111,6 +114,10 @@ function (
});
},

initializeInterTabChannel: function () {
this._interTabChannel = new InterTabChannel();
},

initializeConfig: function () {
return this._configLoader.fetch()
.then(_.bind(this.useConfig, this))
Expand All @@ -119,7 +126,8 @@ function (
.then(_.bind(this.initializeRelier, this))
// channels relies on the relier
.then(_.bind(this.initializeChannels, this))
// fxaClient depends on the relier.
// fxaClient depends on the relier and
// inter tab communication.
.then(_.bind(this.initializeFxaClient, this))
// profileClient dependsd on fxaClient.
.then(_.bind(this.initializeProfileClient, this))
Expand Down Expand Up @@ -197,7 +205,8 @@ function (
initializeFxaClient: function () {
if (! this._fxaClient) {
this._fxaClient = new FxaClient({
relier: this._relier
relier: this._relier,
interTabChannel: this._interTabChannel
});
}
},
Expand All @@ -218,7 +227,8 @@ function (
language: this._config.language,
relier: this._relier,
fxaClient: this._fxaClient,
profileClient: this._profileClient
profileClient: this._profileClient,
interTabChannel: this._interTabChannel
});
}
this._window.router = this._router;
Expand Down
50 changes: 50 additions & 0 deletions app/scripts/lib/channels/inter-tab.js
@@ -0,0 +1,50 @@
/* 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/. */

/**
* This is a special channel that communicates between two
* tabs of the same browser. It uses localStorage to communicate.
*/

'use strict';

define([
'crosstab'
], function (crosstab) {

function InterTabChannel(options) {
options = options || {};
this._crosstab = options.crosstab || crosstab;
}

InterTabChannel.prototype = {
emit: function (name, data) {
// Sensitive data is sent across the channel and should only
// be in localStorage if absolutely necessary. Only send
// data if another tab is listening.
if (this._crosstab.util.tabCount() > 1) {
try {
this._crosstab.broadcast(name, data, null);
} catch (e) {
// this can blow up if the browser does not support localStorage
// or if on a mobile device. Ignore the error.
}
}
},

on: function (name, callback) {
this._crosstab.util.events.on(name, callback);
},

off: function (name, callback) {
this._crosstab.util.events.off(name, callback);
},

clearMessages: function () {
this._crosstab.util.clearMessages();
}
};

return InterTabChannel;
});
75 changes: 54 additions & 21 deletions app/scripts/lib/fxa-client.js
Expand Up @@ -32,6 +32,8 @@ function (_, FxaClient, $, p, Session, AuthErrors, Constants, Channels, BaseReli
this._signUpResendCount = 0;
this._passwordResetResendCount = 0;
this._channel = options.channel;
this._interTabChannel = options.interTabChannel;

// BaseRelier is used as a NullRelier for testing.
this._relier = options.relier || new BaseRelier();
}
Expand Down Expand Up @@ -140,7 +142,11 @@ function (_, FxaClient, $, p, Session, AuthErrors, Constants, Channels, BaseReli
sessionTokenContext: self._relier.get('context')
};

if (self._relier.isFxDesktop()) {
// isSync is added in case the user verifies in a second tab
// on the first browser, the context will not be available. We
// need to ship the keyFetchToken and unwrapBKey to the first tab,
// so generate these any time we are using sync as well.
if (self._relier.isFxDesktop() || self._relier.isSync()) {
updatedSessionData.unwrapBKey = accountData.unwrapBKey;
updatedSessionData.keyFetchToken = accountData.keyFetchToken;
updatedSessionData.customizeSync = options.customizeSync;
Expand All @@ -156,25 +162,38 @@ function (_, FxaClient, $, p, Session, AuthErrors, Constants, Channels, BaseReli
}

Session.set(updatedSessionData);
// Skipping the relink warning is only relevant to the channel
// communication with the Desktop browser. It may have been
// done during a sign up flow.
updatedSessionData.verifiedCanLinkAccount = true;

return Channels.sendExpectResponse('login', updatedSessionData, {
channel: self._channel
}).then(function () {
return accountData;
}, function (err) {
// We ignore this error unless the service is set to Sync.
// This allows us to tests flows with the desktop context
// without things blowing up. In production/reality,
// the context is set to desktop iff service is Sync.
if (self._relier.isSync()) {
throw err;
}
return accountData;
});
if (self._interTabChannel) {
self._interTabChannel.emit('login', updatedSessionData);
}

if (options.notifyChannel !== false) {
return self.notifyChannelOfLogin(updatedSessionData)
.then(function () {
return accountData;
});
}
});
},

// TODO - this should go on the broker when that
// functionality is available.
notifyChannelOfLogin: function (dataToSend) {
// Skipping the relink warning is only relevant to the channel
// communication with the Desktop browser. It may have been
// done during a sign up flow.
dataToSend.verifiedCanLinkAccount = true;

var self = this;
return Channels.sendExpectResponse('login', dataToSend, {
channel: self._channel
}).then(null, function (err) {
// We ignore this error unless the service is set to Sync.
// This allows us to tests flows with the desktop context
// without things blowing up. In production/reality,
// the context is set to desktop iff service is Sync.
if (self._relier.isSync()) {
throw err;
}
});
},

Expand Down Expand Up @@ -326,9 +345,12 @@ function (_, FxaClient, $, p, Session, AuthErrors, Constants, Channels, BaseReli
});
},

completePasswordReset: function (originalEmail, newPassword, token, code) {
completePasswordReset: function (originalEmail, newPassword, token, code,
options) {
options = options || {};
var email = trim(originalEmail);
var client;
var self = this;
return this._getClientAsync()
.then(function (_client) {
client = _client;
Expand All @@ -338,6 +360,17 @@ function (_, FxaClient, $, p, Session, AuthErrors, Constants, Channels, BaseReli
return client.accountReset(email,
newPassword,
result.accountResetToken);
})
.then(function () {
if (options.shouldSignIn) {
// Sync should not notify the channel on password reset
// because nobody will be listening for the message and
// a channel timeout will occur if the message is sent.
// If the original tab is still open, it'll handle things.
return self.signIn(email, newPassword, {
notifyChannel: ! self._relier.isSync()
});
}
});
},

Expand Down
13 changes: 1 addition & 12 deletions app/scripts/lib/url.js
Expand Up @@ -38,16 +38,6 @@ function (_) {
return allowedTerms;
}

/**
* Returns true if given "uri" has HTTP or HTTPS scheme
*
* @param {String} uri
* @returns {boolean}
*/
function isHTTP (uri) {
return /^(http|https):\/\//.test(uri);
}

return {
searchParams: searchParams,
searchParam: function (name, str) {
Expand All @@ -63,8 +53,7 @@ function (_) {
.replace(/\/$/, '')
// search params can contain sensitive info
.replace(/\?.*/, '');
},
isHTTP: isHTTP
}
};
});

6 changes: 5 additions & 1 deletion app/scripts/require_config.js
Expand Up @@ -18,7 +18,8 @@ require.config({
speedTrap: '../bower_components/speed-trap/dist/speed-trap',
md5: '../bower_components/JavaScript-MD5/js/md5',
canvasToBlob: '../bower_components/blueimp-canvas-to-blob/js/canvas-to-blob',
moment: '../bower_components/moment/moment'
moment: '../bower_components/moment/moment',
crosstab: 'vendor/crosstab'
},
config: {
moment: {
Expand Down Expand Up @@ -48,6 +49,9 @@ require.config({
},
sinon: {
exports: 'sinon'
},
crosstab: {
exports: 'crosstab'
}
},
stache: {
Expand Down
4 changes: 3 additions & 1 deletion app/scripts/router.js
Expand Up @@ -80,7 +80,8 @@ function (
language: this.language,
relier: this.relier,
fxaClient: this.fxaClient,
profileClient: this.profileClient
profileClient: this.profileClient,
interTabChannel: this.interTabChannel
}, options || {});

this.showView(new View(options));
Expand Down Expand Up @@ -129,6 +130,7 @@ function (
this.relier = options.relier;
this.fxaClient = options.fxaClient;
this.profileClient = options.profileClient;
this.interTabChannel = options.interTabChannel;

this.$stage = $('#stage');

Expand Down

0 comments on commit 8ba4336

Please sign in to comment.