Closed Bug 1033238 Opened 5 years ago Closed 5 years ago

Cannot revoke Mobile ID permission

Categories

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

defect
Not set

Tracking

(blocking-b2g:2.0+, firefox31 wontfix, firefox32 fixed, firefox33 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)

RESOLVED FIXED
2.0 S6 (18july)
blocking-b2g 2.0+
Tracking Status
firefox31 --- wontfix
firefox32 --- fixed
firefox33 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: freddyb, Assigned: ferjm)

References

Details

(Whiteboard: ft:loop)

Attachments

(1 file)

+++ 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 ;))
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.
Flags: needinfo?(ferjmoreno)
Assignee: nobody → ferjmoreno
Flags: needinfo?(ferjmoreno)
Attached patch v1Splinter Review
Attachment #8454027 - Flags: review?(jed+bmo)
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.
Attachment #8454027 - Flags: review?(jed+bmo) → review+
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 :)
blocking-b2g: --- → 2.0?
(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.
https://hg.mozilla.org/mozilla-central/rev/387dff702fab
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S6 (18july)
blocking-b2g: 2.0? → 2.0+
You need to log in before you can comment on or make changes to this bug.