1033453 - Remember my permissions for PROMPT_ACTION composed permission (permission-read, permission-write) WebAPIs are lost on an app update

Status: RESOLVED FIXED

(Core Graveyard :: DOM: Apps, defect)

Description

[Antonio Manuel Amaya Calvo (:amac)]

Steps:

1. Install a packaged app calling out contacts: {access: read} support
2. Access the contacts API and remember my permissions to allow
3. Update the packaged app on the server
4. Check for updates (manual or automatic)
5. Update the app
6. Check the settings app perm UI or try to access contacts in the updated app

Expected:

The contacts-read permission should be to allow, not ask. So we should retain permissions remembered on an update.

Actual:

The contacts-read permission reverts to ask. So remembering my permissions for allow is lost on an update of the app.

Comment 1

[Antonio Manuel Amaya Calvo (:amac)]

Attachment: V1. Check the correct permission name on the update

The problem here is that I was checking the original permission name (instead of the expanded permission name) to see if the user had granted the permission previously. That works for normal permissions, but won't work for composed permissions, or permissions that are substitued.

Added a test case for this also... better late than never I guess :)

Try run is at https://tbpl.mozilla.org/?tree=Try&rev=356616c57606

Comment 2

[Antonio Manuel Amaya Calvo (:amac)]

Cancelled the try... now I remember why that test wasn't written, contacts-read is privileged and the test installs web apps... Will try with a substituted permission instead and update the patch.

Comment 3

[Antonio Manuel Amaya Calvo (:amac)]

And bar that also... there are only three web prompt permissions, which were the ones I used on the original patch... and none of them are composed in any way.

Comment 4

[Maire Reavy [:mreavy]]

Do we need this for v2.0 (for the mobile Loop app)?

Comment 5

[Antonio Manuel Amaya Calvo (:amac)]

Attachment: v2. Now with tests that actually pass. I hope

Try run at https://tbpl.mozilla.org/?tree=Try&rev=12386f0e1fc9

Comment 6

[Antonio Manuel Amaya Calvo (:amac)]

(In reply to Maire Reavy [:mreavy] (Plz needinfo me, PTO on Jun 23) from comment #4)
> Do we need this for v2.0 (for the mobile Loop app)?

Yes, this is needed for Loop. It's also needed for any privileged app that uses the contacts, device storage, or any other privileged explicit API that has subpermissions (readonly, readwrite...).

Since without this we're losing user information, I think this should be a blocker. Otherwise let me know and I'll ask for approval (the patch is really just changing a parameter in a call, so it's very low risk anyway).

Comment 7

[[:fabrice] Fabrice Desré]

Comment on attachment 8449652 [details] [diff] [review]
v2. Now with tests that actually pass. I hope

Review of attachment 8449652 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/apps/src/PermissionsTable.jsm
@@ +342,5 @@
>                               certified: PROMPT_ACTION
> +                           },
> +                           // This permission doesn't actually grant access to
> +                           // anything. It exists only to check the correctness
> +                           // of web prompt composed permissions.

nit: add " in tests."

Comment 8

[Antonio Manuel Amaya Calvo (:amac)]

Attachment: v3, with nit fixed

r=fabrice

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/bff799535eb7

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/bff799535eb7

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8b04bdcbd916

Comment 12

[Shally Li（leave from Mozilla）]

> Steps:
> 
> 1. Install a packaged app calling out contacts: {access: read} support

Hi Reporter,
    Could you provide the packaged app's file or name for me to verify this bug?

Thank you!

Comment 13

[Antonio Manuel Amaya Calvo (:amac)]

You can try with the FirefoxOS loop app, for example: https://github.com/mozilla-b2g/firefoxos-loop-client/