Skip to content
This repository has been archived by the owner. It is now read-only.

FxA Sync iframe flow #2101

Closed
ryanfeeley opened this issue Feb 11, 2015 · 12 comments
Closed

FxA Sync iframe flow #2101

ryanfeeley opened this issue Feb 11, 2015 · 12 comments
Assignees

Comments

@ryanfeeley
Copy link
Contributor

@ryanfeeley ryanfeeley commented Feb 11, 2015

So that I can connect my Firefox with Sync from more places, as a Firefox user, I want to be able to authenticate (register or login) from an iframe.


stomlinson's list of sub work:

  • A bi-directional WebChannel is needed to communicate with the browser. (#2231, PR #2358)
  • An AuthenticationBroker that can communicate with Sync using the bi-directional WebChannel (#2359, PR #2237)
  • A "Chromeless" UI is needed - one without the X and Firefox logo (#2198, PR #2236)
  • The content server must allow / to be iframed. The iframe's parent origin needs to be checked to ensure we trust the framer. The OAuth flow uses the client_id, then checks the parent origin is the same as is registered for the client_id. This is not an OAuth flow. (#2242, PR #2364)
  • The content server should decide whether to show /signin, /signup, or /settings - this can be done by visiting /. Expected behavior needs clarification. (#2197)
  • Determine the URL API - how does the relier indicate:
    • Chromeless UI
    • Are in an iframe (OAuth reliers use context=iframe)
    • Should use the WebChannel to communicate (webChannelId=sync possibly?)
    • Is using Sync (currently we use service=sync&context=fx_desktop_v1) and no "signout" link should be displayed on settings.
    • A possibility: https://accounts.firefox.com/?service=sync&context=iframe
  • Unlike the normal Sync flow, the "settings" page should be displayed after sign in, and the "account verified" page should be displayed in the original tab after account verification. (#2674, PR #2687)
  • TOS/PP rendering results in a scroll bar in the center of the page - should TOS/PP agreements be loaded in a new tab? YES! (#2351, PR #2643)
  • Links in iframed TOS/PP replace content - (#1968, PR #2337)
  • Channel timeout needs to go - (#2146, PR #2358)
  • Disable the browser's "remember password" after signin/signup. (#2373, PR #2374, https://bugzilla.mozilla.org/show_bug.cgi?id=1173688)
  • Remove the title for sync/iframe (#2565, PR #2593)
  • Pass campaign and entrypoint to the verification page (#2491, PR #2688)
  • Pass uniqueUserId to the verification page to tie together signup/verification metrics (#2491, PR #2756)
  • Determine whether the user is part of metrics in a more deterministic fashion (#2491, #2780)
  • Notify the parent when the content height changes (#2566, PR #2594)
  • Possibly consume GA query parameters and report them to metrics (#2579, PR #2689, PR #2779)
  • Send consumed GA stats to Google (#2782, PR #2787)
  • Prevent sign out link from showing on the settings page for accounts authenticated from firstrun (#2658, PR #2853)
  • Fix partially hidden body (#2669, PR #2675)
  • Notify iframe parent on initial screen load, login, and verification (#2704, PR #2705)
  • Fix padding around marketing snippet on verification page (#2709, PR #2798)
  • send a signal to the parent on successful signup (#2790, PR #2793)
  • send a signal to the parent on unsuccessful signup (#2791)
  • cancel in the merge warning should prevent the signup/signin (#2844, PR #2845)

Bugzilla tracking bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1150231

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Feb 11, 2015

Can we get more info about the use case? Sync is pretty specific to the browser, supporting signin to Sync from arbitrary sites seems like it could be a big ball of yuck.

@ryanfeeley
Copy link
Contributor Author

@ryanfeeley ryanfeeley commented Feb 11, 2015

Only from the stub installer, the onboarding tour, or authorized sites. I had a discussion with Michael Verdi this morning and realized how offering account registration during UI tours is not terribly helpful.

@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Feb 11, 2015

This might also work as a standalone component and not be part of the content server. nevermind

@ckarlof
Copy link
Contributor

@ckarlof ckarlof commented Feb 12, 2015

This could be implemented by the FxAccounts.jsm component in the browser opening up a generic WebChannel to listen to messages from accounts.firefox.com. Bugzilla bug about this: https://bugzilla.mozilla.org/show_bug.cgi?id=998051, but filed before WebChannel landed. It should be fairly straightforward. :)

Open questions:

  • what kind of broker does the enclosing page trigger (Sync vs OAuth)
  • do browser content scripts get added to iframes (this is how WebChannels work)?
@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Mar 24, 2015

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Mar 24, 2015

This comment has been moved to the checklist in the description.

@rfk
Copy link
Member

@rfk rfk commented Apr 26, 2015

The content server should decide whether to show /signin or /signup - this can be done by visiting /.

We may have bikeshedded this already, so appologies if I just don't remember it. But does it make sense to include some sort of verb in the URL for all oauth redirects and reserve / for a user-facing landing page? I'm thinking something generic like /connect or /authenticate or even just /oauth.

I get a vague something-not-quite-right feeling about having the oauth redirect dance land at the root of the site, although I don't feel strongly about it and don't want to derail what you've got going on here.

(Also we'll need an issue in fxa-oauth-server to adjust the meaning of the action parameter in https://github.com/mozilla/fxa-oauth-server/blob/master/docs/api.md#request-parameters-4)

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Apr 27, 2015

But does it make sense to include some sort of verb in the URL for all oauth redirects and reserve / for a user-facing landing page? I'm thinking something generic like /connect or /authenticate or even just /oauth.

I get a vague something-not-quite-right feeling about having the oauth redirect dance land at the root of the site, although I don't feel strongly about it and don't want to derail what you've got going on here.

I had the same thought last night. The firstrun/Sync flow is not going to be an OAuth relier, so sending the user to / seems alright. For OAuth reliers, sending to / does feel like a hack. Sending them to /oauth or /oauth/do-the-right-thing would be better.

shane-tomlinson pushed a commit that referenced this issue Jul 17, 2015
* Reliers can specify Google Analytics query parameters to be logged/sent to
  our metrics infrastructure.
* Consume the following Google Analytics query parameters:
  * utm_campaign
  * utm_content
  * utm_medium
  * utm_source
  * utm_term

fixes #2579
ref #2101
shane-tomlinson pushed a commit that referenced this issue Jul 17, 2015
* Reliers can specify Google Analytics query parameters to be logged/sent to
  our metrics infrastructure.
* Consume the following Google Analytics query parameters:
  * utm_campaign
  * utm_content
  * utm_medium
  * utm_source
  * utm_term

fixes #2579
ref #2101
shane-tomlinson pushed a commit that referenced this issue Jul 17, 2015
* Reliers can specify Google Analytics query parameters to be logged/sent to
  our metrics infrastructure.
* Consume the following Google Analytics query parameters:
  * utm_campaign
  * utm_content
  * utm_medium
  * utm_source
  * utm_term

fixes #2579
ref #2101
shane-tomlinson pushed a commit that referenced this issue Jul 17, 2015
* Reliers can specify Google Analytics query parameters to be logged/sent to
  our metrics infrastructure.
* Consume the following Google Analytics query parameters:
  * utm_campaign
  * utm_content
  * utm_medium
  * utm_source
  * utm_term

fixes #2579
ref #2101
shane-tomlinson pushed a commit that referenced this issue Jul 18, 2015
* Reliers can specify Google Analytics query parameters to be logged/sent to
  our metrics infrastructure.
* Consume the following Google Analytics query parameters:
  * utm_campaign
  * utm_content
  * utm_medium
  * utm_source
  * utm_term

fixes #2579
ref #2101
shane-tomlinson pushed a commit that referenced this issue Jul 20, 2015
* Reliers can specify Google Analytics query parameters to be logged/sent to
  our metrics infrastructure.
* Consume the following Google Analytics query parameters:
  * utm_campaign
  * utm_content
  * utm_medium
  * utm_source
  * utm_term

fixes #2579
ref #2101
@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Jul 30, 2015

There is only one item in this list that does not have a PR, #2791. I'm going to close this out as "this is ready enough to ship."

@rfk
Copy link
Member

@rfk rfk commented Jul 30, 2015

\o/

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants