1010623 - Expose /account/status API via FxAccounts.jsm and FxAccountsClient.jsm

Status: RESOLVED FIXED

(Firefox :: Firefox Accounts, defect)

Description

[Sam Penrose]

When the FxA server returns ERRNO_INVALID_AUTH_TOKEN, we may want to distinguish (as in bug 1004319) between the case where the account has been deleted and the case where the account exists but the token is invalid for some other reason (such as a password change). The server provides /account/status?uid=<current uid> for this purpose. I propose to expose it more-or-less transparently in these two modules for consumption by FxAcountsManager.jsm and elsewhere.

Comment 1

[Sam Penrose]

Attachment: 1010623-account-status.patch

My goal was a straightforward exposing of the server API for shared code. Thanks Jed!

Comment 2

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

Comment on attachment 8422855 [details] [diff] [review]
1010623-account-status.patch

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

This is a very useful patch.

As noted in the comments in the test, I think you just need to nest the second promise inside the resolution of the first.

Just make that tweak.  Otherwise, this looks good to me.  Thanks, Sam!

::: services/fxaccounts/tests/xpcshell/test_accounts.js
@@ +60,5 @@
>  
> +  this.accountStatus = function(uid) {
> +    let deferred = Promise.defer();
> +    deferred.resolve(!!uid && (!this._deletedOnServer));
> +    return deferred.promise;

nice

@@ +522,5 @@
> +      do_check_false(result);
> +    }
> +  );
> +
> +  fxa.setSignedInUser(alice).then(

I think you want to nest this promise inside the onResolve clause of the previous one.  Otherwise they may not resolve in order.

Comment 3

[Sam Penrose]

Attachment: 1010623-account-status.patch

https://tbpl.mozilla.org/?tree=Try&rev=f0b6451627bc

Thanks Jed! Nice catch on the promise chaining.

Comment 4

[Carsten Book [:Tomcat]]

landed this on m-i as https://hg.mozilla.org/integration/mozilla-inbound/rev/75b0548333ca - btw Sam seems your commit message had the wrong bugnumber in it and was metioning bug 1004319 as bug number for the patch. Corrected this now manually :)

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/75b0548333ca

Comment 6

[Chris Karlof [:ckarlof]]

We may need to tweak the /account/status endpoint a bit, but the changes should be minor.

Comment 7

[Sam Penrose]

(In reply to Chris Karlof [:ckarlof] from comment #6)
> We may need to tweak the /account/status endpoint a bit, but the changes
> should be minor.

Angels-on-the-head-of-a-pin territory: /account/status is an implicitly general API that currently returns only {exists: <boolean>} but appears designed for adding attributes. I implemented accountStatus() to just return the boolean. Now that I've landed that, I kind of think accountStatus() should return the blob. If anyone has string opinions LMK, otherwise I may (or may not) update it.

Comment 8

[Chris Karlof [:ckarlof]]

Proposed changes to /account/status here: https://github.com/mozilla/fxa-auth-server/issues/721

It should be backwards compatible with what was implemented here, but this implementation should be updated to support the new functionality: Bug 1011773.