1199258 - signing in to FxA and clicking email prefs on 40.0 /firstrun flow results in blank iframe

Status: RESOLVED FIXED

(www.mozilla.org :: Pages & Content, defect)

Description

[Jonathan Petto [:jpetto]]

STR:

1. navigate to https://www.mozilla.org/en-US/firefox/40.0/firstrun/
2. sign in with existing account
3. click "Communication preferences"
4. click "Email preferences"

Expected: Email preference screen is shown

Actual: iframe is blank due to Load denied by X-Frame-Options

See the original issue in GitHub: https://github.com/mozilla/fxa-content-server/issues/2945#issue-100813304

Comment 1

[Jonathan Petto [:jpetto]]

Suggestion in the GitHub issue [1] is to redirect all users successfully logging in to the embedded iframe to https://accounts.firefox.com/settings.

We are already listening for the login event broadcast from the iframe, so doing a redirect like this would not be difficult to implement.

:cmore :habber - Thoughts on the UX side?


[1] https://github.com/mozilla/fxa-content-server/issues/2945#issuecomment-134669721

Comment 2

[Holly Habstritt Gaal [:Habber]]

Just to be clear... the redirect solution does the redirect immediately after successful signin (Step 2)? Not after they click "communication preferences" (Step 3), correct? If this is true, redirect solution will take users away from the /firstrun URL without them explicitly deciding to exit. At the moment there is no other content on the page that we wish users to interact with. However, there have been requests to display other content on this page (ie: one tertiary link to promote iOS, etc). After they sign in, it's an opportunity to have the user complete a follow-up action or learn something else on this page (or whatever mozilla.org page this is embedded on). Redirecting them too soon could be a missed opportunity. 

A few ideas:
- solve with the redirect solution for now, knowing that we need to come up with a different solution soon that does not take the user away from this page w/ out them explicitly clicking a link to do so

- open 'Email Settings' in new tab (unsure if this is possible from the iframe)

- replace 'communication preferences' link on this screen http://cl.ly/image/1E0W3n0m1f08 with a link that goes here about:preferences#sync, so the user chooses to exit. 


The accounts team (rfeeley & team) should make the call on how the links are displayed in the iframe since they own the product. However, since the iframe needs to stand alone and potentially work when other content on a web page is a priority, I would hesitate from any automatic redirects away from a page where the iframe is embedded that the user wouldn't expect. We have the opportunity to use this iframe on other pages where it is useful to the user, not just on /firstrun.

Comment 3

[Jonathan Petto [:jpetto]]

(In reply to Holly Habstritt Gaal [:Habber] from comment #2)
> Just to be clear... the redirect solution does the redirect immediately
> after successful signin (Step 2)? Not after they click "communication
> preferences" (Step 3), correct?

Correct.

Agreed that immediately redirecting someone who logs in is not great UX. It's a fairly trivial fix on the bedrock side though, and we do need to do *something* to improve the current experience.

We should keep in mind that users with an existing FxA account that also see /firstrun are (ostensibly) rare - though I'd love to see numbers on this.

It seems our best/quickest solutions are either:

1 - Redirect users when they sign in

2 - Hide the post-successful login links in the iframe (as we will want to keep users on future /firstrun pages). Not sure about the dev time/consequences here.

Based on comments in the GitHub issue, the FxA team would like consistent behavior (likely redirecting the entire page to a settings page), but us on the bedrock side may have competing desires.

Can we make an interim call for this specific case, then take the larger FxA/bedrock integration UX question to a different (yet to be created) bug?

The easiest fix is likely the redirect. Who wants to make the call here? :habber? :rfeeley?

Comment 4

[Ryan Feeley [:rfeeley]]

Could we just make the links in the iframe target the top?

Comment 5

[Chris More [:cmore]]

:jpetto: Proceed with a bedrock redirect to accounts.firefox.com/settings when the iframe has broadcasted the "sign in" event to the parent page. Thanks!

Comment 6

[Jonathan Petto [:jpetto]]

Running in to some odd behavior while testing the fix:

* Testing locally (against stomlinson's dev instance)

An immediate redirection after receiving the "login" event from the iframe results in an "Unexpected Error" message in the iframe, though the login appears to be successful.

I can set a 500ms delay before redirecting, which takes care of the error message, but isn't great UX, as the user sees the iframe success screen for a moment and is then redirected.

I can also slide the entire FxA frame up after successful login, then wait 500ms, then redirect. This seems to be the best option so far, but I'm still unsure as to why the FxA frame is throwing an error with an immediate redirect.

- Testing on demo4 (against https://accounts.stage.mozaws.net/)

https://www-demo4.allizom.org/en-US/firefox/40.0/firstrun/

Same behavior/error message as above when doing an immediate redirect.

Also the FxA interface & internals appear to have changed on FxA stage. I can't figure out how to get the iframe to forget me after creating an account or signing in once, meaning I have to create a new profile each time I want to sign up/sign in. I can disconnect my account from the Sync prefs, but that doesn't affect the FxA iframe - it always remembers. Perhaps additional profile tweaks are required for testing against FxA stage?

The iframe slide up + delayed redirect fix is *probably* sufficient, but I'd like to know a bit more regarding the issues above before moving forward.

Comment 7

[Ryan Kelly [:rfkelly]]

> but I'm still unsure as to why the FxA frame is throwing an error with an immediate redirect.

Totally guessing, but perhaps it's causing some in-flight XHR request to be cancelled, similar to what's described here:

  http://stackoverflow.com/questions/1370322/jquery-ajax-fires-error-callback-on-window-unload-how-do-i-filter-out-unload-a

Comment 8

[Shane Tomlinson [:stomlinson]]

(In reply to Ryan Kelly [:rfkelly] from comment #7)
> > but I'm still unsure as to why the FxA frame is throwing an error with an immediate redirect.
> 
> Totally guessing, but perhaps it's causing some in-flight XHR request to be
> cancelled, similar to what's described here:
> 
>  
> http://stackoverflow.com/questions/1370322/jquery-ajax-fires-error-callback-
> on-window-unload-how-do-i-filter-out-unload-a

Jpetto and I went heads down a bit on this yesterday. A cancelled XHR request is exactly what is happening. As soon as the user signs in, the user is redirected to the /settings screen. The settings screen immediately checks the user's sessionToken by calling /session/status. The page begins its redirect while the XHR request is in flight, which causes the Unexpected Error. This has an odd side effect that it also seems to invalidate the user's sessionToken, so when the user is sent to the settings page, they are immediately redirected back to the signin page.

The danger of waiting 500ms to redirect is the XHR request may still not be complete on a slow connection.

A potential solution jpetto and I talked about yesterday involves going old school and watching for a hash change on the FxA iframe.

When FxA is embedded in an iframe, we cannot use pushState to cause a screen transition because this causes the parent frame to redirect. To work around this, we use a hash change. The new screen's name is placed on the URL hash [1].

Perhaps the parent page could watch for hash changes on the child frame, when it changes to settings, then redirect the outer page to FxA's settings page.

I propose a two part solution:
1. The content server "halts" after sign in, it does not redirect to the settings page. This can be changed by changing a single "false" to "true" [2].
2. As soon as the firstrun page receives the "login" message, redirect to FxA's settings.

I think we could cause the outer page to redirect to /settings ourselves, but this removes flexibility from the firstrun page if it ever wants to make changes.

[1] - https://github.com/mozilla/fxa-content-server/blob/d4080dfd539392b7c52547d70f0d7a7ca8a24d11/app/scripts/lib/app-start.js#L580-L587
[2] - https://github.com/mozilla/fxa-content-server/blob/d4080dfd539392b7c52547d70f0d7a7ca8a24d11/app/scripts/models/auth_brokers/first-run.js#L26

Comment 9

[Shane Tomlinson [:stomlinson]]

Another strange case to consider is how to handle users who are authenticated to FxA that re-load the firstrun page. They'll see the settings page.

Do we care?

Comment 10

[Jonathan Petto [:jpetto]]

It doesn't look like the old-school method of monitoring the iframe's hash will work, as access is denied due to differing domains.

I agree that we don't want the iframe changing the URL of the parent frame, as that does paint us into a corner.

If halting the content server after login isn't a huge amount of work and doesn't cause other issues, that solution sounds perfect, as we can simply redirect as soon as the "login" postMessage is received.

(In reply to Shane Tomlinson [:stomlinson] from comment #9)
> Another strange case to consider is how to handle users who are
> authenticated to FxA that re-load the firstrun page. They'll see the
> settings page.
> 
> Do we care?

Though I don't think it's an immediate concern, we should definitely figure out a nice UX for this rare case.

Comment 11

[Chris More [:cmore]]

I have a simple idea. What if we just remove the communication preferences link from the verified email page when it's going through the firstrun flow? Is it really necessary to have that link there for brand new users that can manage it anytime through the Firefox menu?  We could just wrap a conditional around the link(s) on the FxA verified page that don't display when it is embedded on the /firstrun/ flow. 

Thoughts?

Comment 12

[Chris More [:cmore]]

Actually, I mean the logged in screen and not the verification page....

Comment 13

[Ryan Kelly [:rfkelly]]

FWIW, :thumbsup: to Shane's proposal of halting after signin for an initial fix; PR here for context:

  https://github.com/mozilla/fxa-content-server/pull/3021

> What if we just remove the communication preferences link

The difficulty here is we just redesigned the /settings page, so it's not quite as simple as hiding that one link any more.

Longer-term I think you're right Chris, we should show a "you're successfully signed in, good job!" page without outgoing links or settings to fiddle with.  But we'll have to discuss that as a follow-up issue, since it's now a larger piece of work than just hiding links on the existing settings page.

By the way, do we have a sense of how many users are actually encountering this error in production?

Comment 14

[Chris More [:cmore]]

Any movement here? I thought we were working on the logged-in redirect fix.

Comment 15

[Jonathan Petto [:jpetto]]

An update was recently made to the FxA content server [1] that will allow us to safely implement the redirect. According to stomlinson, the fix will go out next week, at which point we can make the (relatively small) change on the bedrock side to redirect users post login.

:stomlinson - Do you know which day next week the FxA fix will make it to production?

[1] https://github.com/mozilla/fxa-content-server/pull/3021/files#diff-9751982e0b5edd17102fe8dfac7e8cb7

Comment 16

[Shane Tomlinson [:stomlinson]]

:jpetto - most likely next Wednesday. You should be able to test demo4 against stomlinson.dev.lcip.org to make sure it works. Be sure to specify `haltAfterSignIn=true` in the query parameters. If it works there, you can push to your prod server before it releases on FxA prod, the extra parameter will not cause any ill effect.

Comment 17

[Jonathan Petto [:jpetto]]

:stomlinson - 

Adding the haltAfterSignIn=true param doesn't seem to work when testing from my local server (127.0.0.1:8111). This is my iframe src:

https://stomlinson.dev.lcip.org/?utm_campaign=fxa-embedded-form&utm_medium=referral&utm_source=firstrun&utm_content=fx-40.0&entrypoint=firstrun&service=sync&context=iframe&style=chromeless&haltAfterSignIn=true

Redirecting immediately after receiving the "login" postMessage results in the same "Unexpected Error" as before.

Do I have to test only on demo4? Should it be working locally?

Comment 18

[Ryan Kelly [:rfkelly]]

IIRC Shane is taking some PTO over this weekend so he might already have headed off.  I had a quick look on stomlinson.dev.lcip.org and it doesn't look like the code to support haltAfterSignIn=true is live on there, although I'm not sure why.  Jon, it may be worth trying on latest.dev.lcip.org instead, unless Shane's box has some additional setup that you need for the test.

Comment 19

[Jonathan Petto [:jpetto]]

(In reply to Ryan Kelly [:rfkelly] from comment #18)
> Jon, it may be worth trying on latest.dev.lcip.org instead, unless 
> Shane's box has some additional setup that you need for the test.

Unfortunately, :stomlinson's box *does* have specific configuration that allows the iframe to be embedded on 127.0.0.1:8111. If latest.dev.lcip.org allows embedding on one of our demo servers, I could give it a try, but development would still be a little painful, as I'd have to deploy each code change to the server to test.

If the train isn't set to land until Wednesday, Monday & Tuesday should give me enough of time to write and test the fix. I'll plan to hold off until Shane is back.

Comment 20

[Shane Tomlinson [:stomlinson]]

Sorry :jpetto, I thought I pulled it in with other tests I was doing. stomlinson.dev.lcip.org is now running the latest master branch, and should now have `haltAfterSignIn` support.

Comment 21

[Jonathan Petto [:jpetto]]

I've got the redirect working successfully both locally (against stomlinson.dev.lcip.org) and on demo4 (against accounts.stage.mozaws.net).

https://www-demo4.allizom.org/en-US/firefox/40.0/firstrun/

(Assuming you have a profile configured to test against FxA stage.)

I don't think we'll want to push the fix to prod before FxA prod gets updated, as the immediate redirect causes a signin against the current FxA prod to fail. As soon as FxA prod is updated, we can merge and push the bedrock side of the fix.

Comment 22

[Jonathan Petto [:jpetto]]

Attachment: GitHub PR

Comment 23

[[github robot]]

Commits pushed to master at https://github.com/mozilla/bedrock

https://github.com/mozilla/bedrock/commit/671fc82f7a34bb9847f4e1d73f8809bb68ee3264
[fix bug 1199258] Redirect fxa signin users on fx40 firstrun.

https://github.com/mozilla/bedrock/commit/dc5ae9fdf52ba259f5b3ffe2102bdd94846bdfbb
Merge pull request #3316 from mozilla/bug-1199258-redirect-after-fxa-signin-fx40-firstrun

[fix bug 1199258] Redirect fxa signin users on fx40 firstrun.