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

VERIFIED FIXED in B2G C4 (2jan on)

Status

P3
normal
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: jsmith, Assigned: alive)

Tracking

unspecified
B2G C4 (2jan on)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-basecamp:+)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
Created attachment 696923 [details]
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.
(Reporter)

Updated

6 years ago
blocking-basecamp: --- → ?
(Reporter)

Updated

6 years ago
Blocks: 801561
(Reporter)

Comment 1

6 years ago
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.
Flags: needinfo?(jparsons)
(Reporter)

Comment 2

6 years ago
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.
(Reporter)

Comment 3

6 years ago
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.
Triage: QAWANTED to confirm if comment 2 / comment 3 is true
Blocking assuming it is true for now
blocking-basecamp: ? → +
Keywords: qawanted
Priority: -- → P3
Target Milestone: --- → B2G C4 (2jan on)
Assignee: nobody → alive
(Reporter)

Updated

6 years ago
QA Contact: jsmith
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.)
Flags: needinfo?(jparsons)
Assignee: alive → arthur.chen
BTW, may I know does the identity feature included in the V1 scope?
(Reporter)

Comment 7

6 years ago
(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.
Stealing because Authur already has a coat ;)
Assignee: arthur.chen → alive
Whatever you end up with it will be a r+, you deserve a coat! ;)
(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.
(Reporter)

Comment 11

6 years ago
(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.
Created attachment 699050 [details]
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.
Attachment #699050 - Flags: review?(timdream+bugs)
Attachment #699050 - Flags: feedback?(alberto.pastor)
Comment on attachment 699050 [details]
https://github.com/mozilla-b2g/gaia/pull/7336

Redirect review
Attachment #699050 - Flags: review?(timdream+bugs) → review?(ferjmoreno)
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.
Attachment #699050 - Flags: review?(ferjmoreno) → review+
I am still testing on this.
I will merge by myself if I finished it, may tomorrow morning. Thanks guys.
(Reporter)

Comment 16

6 years ago
Removing qawanted only cause the patch is already here and about to land, no need to do analysis for blocking at this point.
Keywords: qawanted
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. :(
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Reporter)

Comment 18

6 years ago
(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?
(Reporter)

Updated

6 years ago
Keywords: verifyme
(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?

Updated

6 years ago
Attachment #699050 - Flags: feedback?(alberto.pastor) → feedback+
(Reporter)

Updated

6 years ago
Depends on: 830358
(Reporter)

Comment 21

6 years ago
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...
Status: RESOLVED → VERIFIED
Keywords: verifyme
(Reporter)

Updated

6 years ago
No longer depends on: 830358
You need to log in before you can comment on or make changes to this bug.