Unwanted authData validation

[ghugues]

Issue Description

Parse Server is validating authData whenever the user object is updated. In many cases, this is unwanted and makes a perfectly good request fail.
I only tested this with Facebook, but since the handling of authData seems to be build in a rather generic way, I expect this issue to arise with other auth providers as well.

Steps to reproduce

• Login with Facebook. It will either create a new user and log you in or directly log you in with an existing user.
• You now have a valid session which should in theory allow you to modify the user however you want.
• Wait for the Facebook token to expire (about a month), or simulate it by removing the app from the Facebook website (In Settings -> Apps)
• Your token is no longer valid. Any call to the Facebook API using this token will fail (this is expected).
• Launch your app again and try to update some fields on the the user. Include the authData field in your PUT request. This is what the iOS SDK does, it includes authData in every request and - as far as I know - this can't be disabled without modifying the SDK code.
• Although you have a valid session token, Parse Server attempts to validate the authData, which fails. This in turn makes your save request fail.

Expected Results

As long as I have a valid session, I expect to be allowed to modify custom fields on my user object, regardless of the validity of the authData.

I understand that the main issue here is the fact that the iOS SDK includes the authData in all PUT requests, even if it hasn't changed. I also understand that even with a valid session you can't be allowed to put anything in authData because it would allow you to put the id of a Facebook account you don't own.

Actual Outcome

A very real world outcome is the following:

• You only use Facebook as a login mechanism. You don't care that the token expires after a while because you always have a fresh new token when the user logs in.
• You don't want to show your users the Facebook login screen again.
• You can refresh the token in the background every time they use your app, but if they stop using your app for more than a month, the token will expire without you being able to refresh it. So this is not a solution.
• After a while, all save operations to the User object fail because of this.

Possible fix

If the authData sent in the request is identical to the authData saved in the database, don't perform any validation. You could even go as far as only checking that the id fields of authData are identical and allow any arbitrary change to the token or expiring date without further validation.

Environment Setup

Parse Server version 2.4.1 running locally or on Heroku (tried both)
Node 6.10
MongoDB storage on Mlab.
Tried on iOS only

[flovilmart]

This should have been addressed by this: #3783

[ghugues]

I've been aware of this issue for a while, and I expected #3783 to fix it, which is why I made the update and re-tested. As far as I know this hasn't solved the issue.

From what I understand from your commit, you've juste moved the issue, because the this.handleAuthDataValidation(authData) at the end of RestWrite.prototype.handleAuthData is still executed.

[flovilmart]

Yeah, but the validation should only be executed if the authData has changed, providing the same auth token as the one that created the user (or the last valid update). I can add an additional test with your PUT request to try to reproduce your STR. In the meantime, could you provide the server logs please? (User creation/link with Facebook as well as the failing PUT call?)

[ghugues]

Here is the request:

verbose: REQUEST for [PUT] /api/1/classes/_User/Qp9oiwJKw7: {
  "authData": {
    "facebook": {
      "id": "****************",
      "access_token": "****************",
      "expiration_date": "2017-07-26T18:23:21.881Z"
    },
    "twitter": {
      "id": "****************",
      "consumer_key": "****************",
      "auth_token": "****************",
      "screen_name": "****************",
      "auth_token_secret": "****************",
      "consumer_secret": "****************"
    }
  },
  "timeSpentInApp": {
    "__op": "Increment",
    "amount": 5.137427985668182
  },
  "sessionsCount": {
    "__op": "Increment",
    "amount": 1
  }
} method=PUT, url=/api/1/classes/_User/Qp9oiwJKw7, ...

As far as I understand, the code that checks whether the authData are identical is never executed. Here is the stack trace that I have after some quick printing:

line call
201  RestWrite.prototype.validateAuthData
230  return this.handleAuthData(authData);
278  RestWrite.prototype.handleAuthData
290  results.length: 1
     this.query: {"objectId":"Qp9oiwJKw7"}
339  return this.handleAuthDataValidation(authData);

[ghugues]

Just tried without the twitter auth just in case, same problem. Just put a print in validateAuthData of Adapters/Auth/facebook.js you will see that it's always executed, even if the tokens are the same.

So the reason the test is not executed is probably the if (!this.query). Maybe it should be if (this.query) ? Because when i do a login this.query is nil, but when I do an update it's not.

I've never looked at Parse Server code before, so I might be wrong...

[flovilmart]

Right I get it now. The skip is on login only and not in the case of the PUT. Are you willing to provide a fix for it? That should not be too hard

[ghugues]

Well I would definitely like to contribute, but I don't think I can... I'm not a Javascript developer. I can read and understand the general idea, but I don't think I can fix it.

[flovilmart]

You actually managed to find the problem :) with a few trial an error, I'm sure you can manage it :) worst case, I can do it in the next days, but that's a good opportunity to learn ;)

[ghugues]

I still don't understand how you differentiate the login from the PUT request. Is this just the this.query check ? Also I'm not sure checking for identical authData here will solve the entire problem.

Let's say you have 2 devices connected with the same Facebook account, each with a different token (but the same Facebook id). One of the device will not have the right token and it will generate a token check on every update. If the token of this device expires, then the device is locked out until a new token is obtained.

Shouldn't the process of comparing 2 authData be moved to the auth adapter ? The validateAuthData of the adapter could be given 2 authData (the one from the database and the one from the request) and then decide if an update is allowed. In the case of Facebook for example you could decide that checking the token is necessary only when the id changes (including from null to something). If the id is the same, changing the token would always be allowed.

In RestWrite.js, this would make thing simpler because you would call handleAuthDataValidation all the time. But this would change the AuthAdapter interface, so this may break things elsewhere...

I've only looked at Parse Server code today, so I'm going to pass on pull requests for now, but I'll probably work more with Parse Server in the following weeks or months, so when I understand better how it works I'll consider fixing some bugs. What would really help though is an explanation of how all the components work together. There is documentation in the code that explain what each method does, but nothing that gives you the big picture.

[flovilmart]

But this would change the AuthAdapter interface, so this may break things elsewhere...

That would imply having that logic in all auth adapters, making some more vulnerable than others. In general, the token should be valid when updated to the database. With the latest fix, this should cover that properly.

Starting now, and on iOS, if you send a valid auth token, this will update the DB. If you send an invalid auth token and that token is still the same as the previous one, the request will not fail.

[ghugues]

Thanks a lot for the fix! There are still some corner cases that will not work, but as you say, this will cover the main case where the token sent is the same as the one in database.

I have a question though: What happens if you try to login with stale authData (invalid token, but same as the one in the database)? I would expect the login to fail in this case for security reasons (I haven't tested, but I'll try to when I get the time).

[flovilmart]

This should be supported, and was actually the 1st supported case.

[ghugues]

Yes, I just tested. I'm actually saying it shouldn't work.
Both in 2.4.1 and 2.4.2, if you make the following request (aka login) :

curl -sv -H "X-Parse-Application-Id:********" -H "Content-Type: application/json" -d @Desktop/request.json -X POST https://path/to/api/users | json

Where request.json is

{"authData": {
    "facebook": {
      "id": "********",
      "access_token": "********",
      "expiration_date": "********"
    }
}}

Then it will always succeed if you pass the same authData as the one in database, even if the token has been revoked because the token is not validated.

I think you should differentiate two scenarios :

• A login call, where the Facebook token is the authentication mechanism and should be validated every time (regardless of the value stored in the database). A revoked Facebook token should not allow you to login.
• An update call, where the authentication is done with the Parse session token, and where it should be accepted that if the token hasn't changed (although it may be invalid), there is no need to validate it and we can let the request succeed.

But we can leave it as is it. To be honest, your fix on the update call was important to me because it avoids locking out users. This on the other hand is much less important. The odds of someone getting their hands on the token event if it's revoked are pretty slim anyway.

[ghugues]

Just saw on your original pull request (#3783) that this was by design. Don't really understand why though.

[flovilmart]

That was probably overlooked at the 1st PR, with the intended behaviour on PUT and not on login.