829170 - When firing a web activity within a trusted UI, we need to minimize the trusted UI, not leave the trusted UI on top of the app the activity goes to

Status: VERIFIED FIXED

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

Description

[krupa raj[:krupa]]

Attachment: screenshot

steps to reproduce:
1. Log into marketplace-dev on your unagi phone
2. Search for 'Private Yacht' on your phone.
3. Click on Purchase
4. Set up your PIN and confirm it
5. Send SMS screen loads

expected behavior:
User can send SMS as instructed to complete the purchase

observed behavior:
The PaymentFlow persists over the SMS screen blocking the user from sending the SMS. 

See attached screenshot.

Comment 1

[Jason Smith [:jsmith]]

Crap. This is what I worried about introducing this feature. I'm probably guessing marketplace is using a web activity here to do this.

Moved to Gaia as this allows the trusted UI to end up on top of an app, which is a sec-bug. Noming to block.

Comment 2

[Jason Smith [:jsmith]]

Alive and Alberto - What's going wrong here?

Comment 3

[Jason Smith [:jsmith]]

Note - I don't think this is a marketplace bug. This looks like poor handling of web activities within a trusted UI.

Comment 4

[Jason Smith [:jsmith]]

Blocking rationale:

This prevents completion of the *true* payment flow and it violates the intention of the trusted UI - we end up on top of an app. Without this fixed, marketplace's payment flow will not work fully with use SMS (which is needed for the payment provider we are using).

Comment 5

[Jason Smith [:jsmith]]

Talked with Fernando over IRC about this.

Looks like what I've stated above is correct - we aren't handling a web activity to send a SMS correctly with a trusted UI. Definitely a Gaia bug with trusted UI.

Comment 6

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

Is this a regression? Does every app are going to be purchased needs to send a SMS?

Comment 7

[Jason Smith [:jsmith]]

(In reply to Alive Kuo [:alive] from comment #6)
> Is this a regression? Does every app are going to be purchased needs to send
> a SMS?

I don't think this is a regression - I don't think this ever worked. And yes, any app for v1 that does a payment for v1 that needs to do a SMS challenge will need to do this.

Can't remember the cases when a SMS challenge happens. But it is part of the core payments flow.

Comment 8

[alberto.pastor]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/7546

Pointer to Github pull-request

Comment 9

[Dylan Oliver [:doliver]]

blocking+ for breaking the payment flow

Comment 10

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

Comment on attachment 700949 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/7546

Looks simple fix, r=me if you have tested it(I don't how to purchase now).
If you are Lab here, could you come to me to show the change?

Comment 11

[Jason Smith [:jsmith]]

Generalizing the bug after talking with Alberto. A really easy way to reproduce this:

1. Go to marketplace
2. Sign in with persona
3. Pull down the notification bar
4. Hit settings

Result - trusted UI remains on top

Comment 12

[alberto.pastor]

https://github.com/mozilla-b2g/gaia/commit/a43285652a8321588f5344dc80014de5d4aa02ef

I didn't find a way to test it with the exact steps the bug describes (payment + sms), so after talking to Jason, QA team willl ensure this patch fixes that scenario.

Thanks!

Comment 13

[Jason Smith [:jsmith]]

Krupa - I don't have the ability to test payments right now on device with the *real* flow. Can you test this with this patch applied?

You can verify if you've got this patch if the test in comment 11 works in the build.

Comment 14

[Jason Smith [:jsmith]]

Krupa mentions in the other bug that this patch doesn't work for her with the SMS app. I then tested this with https://bugzilla.mozilla.org/show_bug.cgi?id=829170#c11 and reproduce the bug with those steps.

Am I running the right build? I can't tell - I could've sworn https://bugzilla.mozilla.org/show_bug.cgi?id=829170#c11 was tested when creating this patch. Alberto - Any ideas?

Comment 15

[Jason Smith [:jsmith]]

Okay I definitely confirm the patch here didn't fix the bug with an even more recent build. Followup bug coming.

Comment 16

[Jason Smith [:jsmith]]

Now it's working on 1/15 build.