825806 - Holding the home button to go to the card view when a trusted UI is about to close and returning to the app - trusted UI renders on top of the app that was recently closed

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System, defect, P3)

Description

[Jason Smith [:jsmith]]

Attachment: Trusted UI in an Incorrect Context

Device: Unagi
Build: 1/1/2013 B2G 18

Example Steps (although there's other ways to reproduce this):

1. Enable identity on your device (dom.identity.enabled)
2. Go to http://www.se.rit.edu/~jds2501/persona_observer_test.html
3. Login with an existing persona account
4. When "finishing sign in" appears, open the task manager and wait
5. Select the card to return to the app

Expected:

You should return to the app and be signed into persona.

Actual:

See screenshot. The dialog ends up failing to close, which reverts itself back to the beginning of the sign in process, even though you did sign in. It's almost as if the request for sign in for persona restarted again when this bug occurs (i.e. login starts all over again). On top of that, the trusted UI dialog ends up top of an app, which is incorrect - a trusted UI context is not allowed to end on top of anything but the homescreen - the user can only "trust" this content on top of an app that web content cannot change. So the fact that the trusted UI dialog ended up on top of another app is a violation of user trust to know that it's a trusted context.

Comment 1

[Jason Smith [:jsmith]]

This could potentially be a core identity issue and a trusted UI issue. Asking Jed for input to see if he has any ideas why persona is doing a restart of the sign in process if the dialog closes while not in view.

Comment 2

[Jason Smith [:jsmith]]

I am curious if a bug like this could end up causing a potential restart of a payment successfully made from the beginning. It might be worthwhile to see if this case could happen during an in-app payment.

Comment 3

[Jason Smith [:jsmith]]

Here's the scenario I'm concerned about that might warrant blocking btw:

1. User says "Hey I want to buy this item in this app"
2. User completes the payment process and immediately hits home to go back to doing other things before the trusted UI finishes closing
3. Trusted UI closes in the background
4. User returns to the app that had the trusted UI up
5. User sees the trusted UI up - they then think, "Hey I thought I bought that thing, but it looks like the UI is still up saying I didn't"
6. User proceeds to buy the item again

Result - the user just got footgunned into spending double the amount for an in-app payment because of a mistake they made previously.

Also - if I try the same test case with instead hitting the home button to go to the homescreen instead, the trusted UI will persist and restart the sign in process again, but it will end up on top of the homescreen. So there's definitely potentially two issues here - an identity issue for knowing how to clean up after you complete a sign in and a trusted UI issue. Or the symptoms may be deriving purely from the trusted UI.

Comment 4

[Joe Cheng [:jcheng] (please needinfo)]

Triage: QAWANTED to confirm if comment 2 / comment 3 is true
Blocking assuming it is true for now

Comment 5

[Ben Adida [:benadida]]

As best as I can tell, this is a trusted UI issue, not specific to identity. Identity doesn't do anything to reopen the trusted UI, that must be something that happens as part of the trusted UI's recovery from app-switch. (And I have a feeling that new trusted UI is wired up all wrong.)

Comment 6

[Arthur Chen [:arthurcc][inactive after 6/30]]

BTW, may I know does the identity feature included in the V1 scope?

Comment 7

[Jason Smith [:jsmith]]

(In reply to Arthur Chen [:arthurcc] from comment #6)
> BTW, may I know does the identity feature included in the V1 scope?

Yes it is. It's not prefed on, but will be - probably will land the patch tomorrow to pref it on.

Comment 8

[(inactive after 6/18) Alive Kuo [:alive]]

Stealing because Authur already has a coat ;)

Comment 9

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Whatever you end up with it will be a r+, you deserve a coat! ;)

Comment 10

[Josh Carpenter [:jcarpenter]]

(In reply to Jason Smith [:jsmith] from comment #3)
> Result - the user just got footgunned into spending double the amount for an
> in-app payment because of a mistake they made previously.

Discussed this in Berlin with Alive, jason and Alberto, and AFAIK the Marketplace explicitly prevents double-charges. This would remain a potential problem issue w/ third party payment flows, though.

Comment 11

[Jason Smith [:jsmith]]

(In reply to Josh Carpenter [:jcarpenter] from comment #10)
> (In reply to Jason Smith [:jsmith] from comment #3)
> > Result - the user just got footgunned into spending double the amount for an
> > in-app payment because of a mistake they made previously.
> 
> Discussed this in Berlin with Alive, jason and Alberto, and AFAIK the
> Marketplace explicitly prevents double-charges. This would remain a
> potential problem issue w/ third party payment flows, though.

For context to those reading this - that means buying a paid app won't hit this problem. But making an in-app payment would.

Comment 12

[(inactive after 6/18) Alive Kuo [:alive]]

Attachment: https://github.com/mozilla-b2g/gaia/pull/7336

Patch v1:

The root cause in this bug is card view needs to stay at homescreen app,
which clear the _lastDisplayedApp flag.
When the login finished, identifier asks trusted ui to close the dialog,
but because the _lastDisplayedApp is wrong now, it fails to either remove the iframe nor reopen the app.

BTW, I really don't like the way how trusted UI cowork with Window Manager...BUT anyway.

Comment 13

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 699050 [details]
https://github.com/mozilla-b2g/gaia/pull/7336

Redirect review

Comment 14

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 699050 [details]
https://github.com/mozilla-b2g/gaia/pull/7336

r=me

I agree with Alberto's comment in the PR. You could use the 'homescreen.manifestURL' setting to check the origin instead of checking for the 'homescreen' class.

Comment 15

[(inactive after 6/18) Alive Kuo [:alive]]

I am still testing on this.
I will merge by myself if I finished it, may tomorrow morning. Thanks guys.

Comment 16

[Jason Smith [:jsmith]]

Removing qawanted only cause the patch is already here and about to land, no need to do analysis for blocking at this point.

Comment 17

[(inactive after 6/18) Alive Kuo [:alive]]

https://github.com/mozilla-b2g/gaia/commit/34fcab4e44e95f89c2a8f192f169cbdb94dc8459

Please NOTE that we still have some trouble if there are two apps using trusted UI at the same time and switch to cards view now. Alberto I look forward that you could work with platform guys to resolve this.
Currently identifier event isn't firing to a specific frame. It's bad. :(

Comment 18

[Jason Smith [:jsmith]]

(In reply to Alive Kuo [:alive] from comment #17)
> https://github.com/mozilla-b2g/gaia/commit/
> 34fcab4e44e95f89c2a8f192f169cbdb94dc8459
> 
> Please NOTE that we still have some trouble if there are two apps using
> trusted UI at the same time and switch to cards view now. Alberto I look
> forward that you could work with platform guys to resolve this.
> Currently identifier event isn't firing to a specific frame. It's bad. :(

Should we file a separate bug on this?

Comment 19

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Jason Smith [:jsmith] from comment #18)
> (In reply to Alive Kuo [:alive] from comment #17)
> > https://github.com/mozilla-b2g/gaia/commit/
> > 34fcab4e44e95f89c2a8f192f169cbdb94dc8459
> > 
> > Please NOTE that we still have some trouble if there are two apps using
> > trusted UI at the same time and switch to cards view now. Alberto I look
> > forward that you could work with platform guys to resolve this.
> > Currently identifier event isn't firing to a specific frame. It's bad. :(
> 
> Should we file a separate bug on this?

We should. I plan to file a platform bug on this. Do you know who is the guy in platform to make persona work with gaia?

Comment 20

[(inactive after 6/18) Alive Kuo [:alive]]

https://bugzilla.mozilla.org/show_bug.cgi?id=828182 filed.

Comment 21

[Jason Smith [:jsmith]]

Well - it looks like the case with the card view works well, but this caused a nasty regression if the user hits the home button instead. See bug 830358 for a followup.

I'll mark as verified as the original bug appears to be fixed, but the regression here is kinda sorta bad...