verifyCredentials does not detect suspension

[jsha]

When a BtUser gets suspended, verifyCredentials does not set the deactivatedAt field, because /account/verify_credentials returns success. verifyCredentials should also do a user lookup to check for suspension and set the deactivatedAt fiedl as appropriate.

Similarly / alternately, update-users.js should store deactivatedAt on the BtUsers table when it notices a suspension.

[jsha]

Also as part of this:

• move verifyCredentials into its own file, it doesn't really belong in setup.js. It doesn't have to be a method on BtUser either, it can take BtUser as an argument.
• Make verifyCredentials return a Promise.

[cpu]

Similarly / alternately, update-users.js should store deactivatedAt on the BtUsers table when it notices a suspension.

It looks like updateUsersCallback is calling deactivateTwitterUser when it finds a uid that is "probably suspended". That function sets the deactivatedAt field on the matched TwitterUser

Does your comment reflect a need to do the same thing for a BtUser record if it can be found for the uid?

[jsha]

Yes, although since verifyCredentials clears the deactivatedAt field, it needs to be fixed first. In more detail: If /verify_credentials.json returns success (not deactivated or revoked), then we should proceed to call /users/show.json (or lookup.json, I forget which is better here). That lookup will return, I think, 403 or 404 if the user is suspended.

Probably once verifyCredentials does this right, it's not important to do it in update-users, since we'll call verifyCredentials on the first failed call for that user.

[cpu]

Ahhh. Ok I think I understand what needs to happen here. I'll get my environment working fully and then take a crack at it.

[cpu]

From my perspective I think /users/show.json is the correct choice for the 2nd lookup to detect the suspension. lookup.json is for bulk requests and we're checking one uid.

The api docs for show don't mention suspension or error return codes :crying_cat_face:

@jsha What do you think the best process is here to test?

[cpu]

Ok, update on this one. I did the following:

1. Created a new account @cpu_test_1
2. Signed up to my local dev blocktogether instance using that account
3. Got the account suspended (this turned out to be harder than you'd think)
4. Modified verifyCredentials to issue an additional /users/show.json request after a successful verify_credentials.json call to attempt to determine if the account is suspended or not.
5. Observed the response behaviour for @cpu_test_1 and compared it to a non-suspended account I had also enrolled in the dev instance.

Turns out that the show request does not return a 404 or any other error status for suspended accounts either. The behaviour was identical for a suspended and a non suspended account. For @cpu_test_1 the return object includes 'suspended': true. It seems like that's the best way for Blocktogether to detect suspended accounts in the verifyCredentials call.

I'll submit a pull req for this shortly.