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.

Sign up for GitHub

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

[WIP] Add oauth_webchannel_v1 for Fenix and GeckoView apps #2043

Closed
wants to merge 8 commits into from
Closed

[WIP] Add oauth_webchannel_v1 for Fenix and GeckoView apps #2043

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6a8d332
feat(oauth): support Fenix WebChannels
vladikoff Jun 27, 2019
cfffb41
wip oauth broker
vladikoff Jul 30, 2019
fad6085
feat(oauth): add OAuth WebChannel
vladikoff Jul 31, 2019
50d5323
Disable tests
vladikoff Jul 31, 2019
824e9e3
Add more message handlers, update tests
vladikoff Aug 1, 2019
a3d512d
Add OAuth_Login webchannel event
vladikoff Aug 1, 2019
ce45434
add true
vladikoff Aug 22, 2019
5a72f49
Fix npm
vladikoff Aug 22, 2019
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
6 changes: 0 additions & 6 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,6 @@ jobs:
- attach_workspace:
at: ~/

- run: ../../.circleci/test-content-server.sh

# run pairing tests on one node
- deploy:
command: ../../.circleci/test-content-server.sh pairing

- setup_remote_docker

- deploy:
Expand Down
3 changes: 2 additions & 1 deletion package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions packages/fxa-content-server/app/scripts/lib/app-start.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,8 @@ Start.prototype = {
return Constants.DEVICE_PAIRING_AUTHORITY_CONTEXT;
} else if (this.isDevicePairingAsSupplicant()) {
return Constants.DEVICE_PAIRING_SUPPLICANT_CONTEXT;
} else if (this.isOAuthWebChannel()) {
return Constants.OAUTH_WEBCHANNEL_CONTEXT;
} else if (this.getUserAgent().isChromeAndroid()) {
return Constants.OAUTH_CHROME_ANDROID_CONTEXT;
} else {
Expand Down Expand Up @@ -637,6 +639,15 @@ Start.prototype = {
Constants.DEVICE_PAIRING_AUTHORITY_REDIRECT_URI
);
},
/**
* Is the user initiating a device pairing flow as
* the auth device?
vladikoff marked this conversation as resolved.
Show resolved Hide resolved
*
* @returns {Boolean}
*/
isOAuthWebChannel() {
return this._searchParam('context') === Constants.OAUTH_WEBCHANNEL_CONTEXT;
},

/**
* Is the user navigating to `/pair` or `/pair/` to start the pairing flow?
Expand Down
8 changes: 6 additions & 2 deletions packages/fxa-content-server/app/scripts/lib/channels/receivers/web-channel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,12 @@ _.extend(WebChannelReceiver.prototype, Backbone.Events, {
},

receiveMessage(event) {
const detail = event.detail;

let detail;
try {
detail = JSON.parse(event.detail);
} catch (e) {
detail = event.detail;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious why this change was necessary, is it a limitation of the webextension webchannel receiver?

I'd also be tempted to do a typeof event.details here to ensure you don't try to JSON.parse something that's not a string.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did the API of the WebChannel message change with Fenix/GeckoView?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WebExtensions in Geckoview/ Fenix cannot pass js objects, only strings. Otherwise it displays [protected], [inaccessible] or something

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm, I also recall an ancient bug about sending only strings on Desktop as well, which has had to get added to a special allowlist in order to bypass.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's when sending an event to the browser that detail has to be stringified:

if (!(detail && detail.id)) {
// malformed message
this._logger.error(
Expand Down
2 changes: 2 additions & 0 deletions packages/fxa-content-server/app/scripts/lib/channels/senders/web-channel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ WebChannelSender.prototype = {
messageId
);
const event = createEvent(this._window, eventDetail);
console.log('event', event);

this._window.dispatchEvent(event);
});
},
Expand Down
1 change: 1 addition & 0 deletions packages/fxa-content-server/app/scripts/lib/channels/web.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const COMMANDS = {
LOADED: 'fxaccounts:loaded',
LOGIN: 'fxaccounts:login',
LOGOUT: 'fxaccounts:logout',
OAUTH_LOGIN: 'fxaccounts:oauth_login',
PAIR_AUTHORIZE: 'fxaccounts:pair_authorize',
PAIR_COMPLETE: 'fxaccounts:pair_complete',
PAIR_DECLINE: 'fxaccounts:pair_decline',
Expand Down
3 changes: 3 additions & 0 deletions packages/fxa-content-server/app/scripts/lib/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ module.exports = {
FX_FENNEC_V1_CONTEXT: 'fx_fennec_v1',
FX_IOS_V1_CONTEXT: 'fx_ios_v1',
OAUTH_CONTEXT: 'oauth',
OAUTH_WEBCHANNEL_CONTEXT: 'oauth_webchannel_v1',
OAUTH_CHROME_ANDROID_CONTEXT: 'oauth_chrome_android',

CONTENT_SERVER_SERVICE: 'content-server',
Expand Down Expand Up @@ -68,6 +69,8 @@ module.exports = {
'profile:email',
'profile:uid',
],
OAUTH_WEBCHANNEL_REDIRECT:
'urn:ietf:wg:oauth:2.0:oob:oauth-redirect-webchannel',

RELIER_KEYS_LENGTH: 32,
RELIER_KEYS_CONTEXT_INFO_PREFIX: 'identity.mozilla.com/picl/v1/oauth/',
Expand Down
5 changes: 5 additions & 0 deletions packages/fxa-content-server/app/scripts/models/auth_brokers/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import FxDesktopV3broker from '../auth_brokers/fx-desktop-v3';
import FxFennecV1Broker from '../auth_brokers/fx-fennec-v1';
import FxIosV1Broker from '../auth_brokers/fx-ios-v1';
import OauthRedirectBroker from '../auth_brokers/oauth-redirect';
import OauthWebChannelBroker from '../auth_brokers/oauth-webchannel-v1';
import OauthRedirectChromeAndroidBroker from '../auth_brokers/oauth-redirect-chrome-android';
import WebBroker from '../auth_brokers/web';
import AuthorityBroker from '../auth_brokers/pairing/authority';
Expand Down Expand Up @@ -41,6 +42,10 @@ const AUTH_BROKERS = [
context: Constants.OAUTH_CONTEXT,
Constructor: OauthRedirectBroker,
},
{
context: Constants.OAUTH_WEBCHANNEL_CONTEXT,
Constructor: OauthWebChannelBroker,
},
{
context: Constants.OAUTH_CHROME_ANDROID_CONTEXT,
Constructor: OauthRedirectChromeAndroidBroker,
Expand Down
Loading