1033238 - Cannot revoke Mobile ID permission

Status: RESOLVED FIXED

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

Description

[Frederik Braun [:freddy] (OOO until August 18th)]

+++ This bug was initially created as a clone of Bug #1023266 +++

In a clean state (i.e. no cached assertions) setting the permission to "deny" in the settings app still allows calling navigator.getMobileIdAssertion(), which gives me the dialog. Is this intended?

The backend service doesn't seem to work with my test app, so I can not currently check the impact in the later application state (when the phone number has been verified and is cached within gecko), so this remains to be tested (help is appreciated ;))

Comment 1

[Maire Reavy [:mreavy]]

Hi Fernando -- Please see Frederik's question above, "This bug was initially created as a clone of Bug 1023266.  In a clean state (i.e. no cached assertions) setting the permission to "deny" in the settings app still allows calling navigator.getMobileIdAssertion(), which gives me the dialog. Is this intended?" 

Since you were the owner of Bug 1023266, you seem best qualified to answer Frederik's question and provide next steps here.  Thanks.

Comment 2

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v1

Comment 3

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Comment on attachment 8454027 [details] [diff] [review]
v1

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

It appears that a variable definition has been erroneously removed.  Make sure that's the way you want it and r=me.

::: services/mobileid/MobileIdentityManager.jsm
@@ +797,5 @@
>      log.debug("getMobileIdAssertion ${}", aPrincipal);
>  
>      let uri = Services.io.newURI(aPrincipal.origin, null, null);
>      let principal = securityManager.getAppCodebasePrincipal(
> +      uri, aPrincipal.appId, aPrincipal.isInBrowserElement);

As one of your reviewers for Bug 988469, I'm sorry I missed this typo.

@@ -863,5 @@
>          // before generating and sharing the assertion.
>          // If we've just prompted the user in the previous step, the permission
>          // is already granted and stored so we just progress the credentials.
>          if (creds) {
> -          let permission = permissionManager.testPermissionFromPrincipal(

I think it is an error to remove this.  It leaves 'permission' (line 897) undefined.

Comment 4

[Fernando Jiménez Moreno [:ferjm]]

Thanks Jed!

(In reply to Jed Parsons (use needinfo, please) [:jedp, :jparsons] from comment #3)
> 
> @@ -863,5 @@
> >          // before generating and sharing the assertion.
> >          // If we've just prompted the user in the previous step, the permission
> >          // is already granted and stored so we just progress the credentials.
> >          if (creds) {
> > -          let permission = permissionManager.testPermissionFromPrincipal(
> 
> I think it is an error to remove this.  It leaves 'permission' (line 897)
> undefined.

I moved the definition to line 804 :)

Comment 5

[Fernando Jiménez Moreno [:ferjm]]

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

Comment 6

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

(In reply to Fernando Jiménez Moreno [:ferjm] from comment #4)
> > I think it is an error to remove this.  It leaves 'permission' (line 897)
> > undefined.
> 
> I moved the definition to line 804 :)

Whoops!  So you did.  So sorry.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/387dff702fab

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a8b32e56b4f0