Field email is not removed from Parse.User for unauthorized requests.

[johanarnor]

Issue Description

A user is fetched by another user, thus an "unauthorized" request. Even though it is stated that since 2.3.0, the email field on the user it is still present in the response.

Steps to reproduce

• Don't specify userSensitiveFields, i.e. let it default to email only.
• Make an unauthorized request to fetch a user.
• Read the email property of the returned user.

Expected Results

The email property should be stripped.

Actual Outcome

The email property is still present.

Environment Setup

• Server

  • parse-server version : 2.3.6
  • Operating System: Linux
  • Hardware: Heroku
  • Localhost or remote server?: Localhost/Heroku - same result

• Database

  • MongoDB version: 3.0.12
  • Storage engine: MMAPv1
  • Localhost or remote server: mLab

Logs/Trace

When the userSensitiveFields field is not set, the userSensitiveFields array (in config) is split up on characters:
userSensitiveFields: [ 'e', 'm', 'a', 'i', 'l' ],

This makes the cleanup removes fields e, m, etc...

When specifying userSensitiveFields to e.g. config.userSensitiveFields = ['test'], the userSensitiveFields array is represented correctly:
userSensitiveFields: [ 'test', 'email' ],

[acinader]

I can't reproduce this (yet).

1. Can you tell me what version of node you're using so I can test with that
2. Can you run the unit tests? If I understand you correctly, then this test: https://github.com/ParsePlatform/parse-server/blob/master/spec/UserPII.spec.js#L78
   should fail? (as well as some other's in that suite?).

[johanarnor]

@acinader

1. I'm currently using node 7.1.0.
2. How are the unit tests designed to run? I just copied the testfile, made some configuration at the top and runed it towards my local parse-server. I'm having a lot of trouble with Error: There is no current user user on a node.js server environment. though.

Yes, it's correct that the given test should fail for me.

[acinader]

well, I used nvm to get and use 7.1.0 and then just npm test focused on

fdescribe('Personally Identifiable Information', () => {

and it passed without error.

To do that yourself, clone parse-server onto your box, cd into that directory, check that you're on the same node version. Then npm install and focus on the tests by editing spec/UserPII.spec.js and putting an f in front of describe('Personally Identifiable Information', () => { on line 12 to focus on those tests, and then run npm test. Do they pass?

If so, the next thing you can do is cd into your dev directory with your code that depends on parse-server and then npm link your local development version of parse-server that we just ran npm test in. That should help us figure out what is going on? maybe? hopefully? ;)

To do this, cd into your dev directory and then npm link /THE/PATH/TO/parse-server

Don't forget to npm unlink parse-server && npm install when you are done.

....futz....futz...futz, ok I am now npm linked and I added some debug output in RestQuery.js starting at line 422:

  console.log('the config:', config.userSensitiveFields);
  for (const field of config.userSensitiveFields) {
    console.log('field', field);
    delete result[field];
  }

Then i transpile parse-server by running npm run build in the parse-server directory and then i curl my server with:

curl -H "X-Parse-Application-Id: OH-WOULDNT-YOU-LIKE-TO-KNOW" -G --data-urlencode 'limit=1' http://localhost:XXXX/parse/classes/_User

and I see the expected output:

info: parse-server running on port XXXX.
the config: [ 'email' ]
field email

[johanarnor]

Many thanks for the detailed answer!

The tests pass but when I'm pointing my dev repo to use the parse-server source instead (via npm link), I get the following output.

the config: [ 'e', 'm', 'a', 'i', 'l' ]
field e
field m
field a
field i
field l

Here is some more info about our config though. When running the server we set the config in the following way:

var ParseServer = require('parse-server').ParseServer
var program = require('parse-server/lib/cli/utils/commander').default
...
var options = program.getOptions()
console.log(typeof options.userSensitiveFields) // string
// setting more options below
...
var parse = new ParseServer(options)
...

When "not specifying" userSensitiveFields, I in fact pass a string. If I set options. userSensitiveFields = undefined things works as expected.

I don't honestly know why we use the config pattern above, since I find no examples of it in the current parse-server documentation. Wouldn't it make more sense to build the options from an empty object instead?

Also, there seems to be no way to turn cleaning of the email field off? What is the reason behind that? We currently have strict ACLs on our User objects, meaning that no regular user can read another user, only admin users can. This introduces a lot of unnecessary refactoring for us, since the admin users no longer can read a user's email.

[johanarnor]

If a string is supplied the resulting array will be an array of chars. Strings as well as Arrays have a concat method.

https://github.com/ParsePlatform/parse-server/blob/master/src/ParseServer.js#L159

[acinader]

ok. that helps. Take a look at #3599.

I don't use the cli to configure so not sure how to test. Can you try it out and see if it fixes for you?

Here's the discussion around the email field: #3556

I do agree that we should do something at this point.

[johanarnor]

Perfect, #3599 solves my issue.

Thanks for the link to the discussion. I'll follow it closely.

A question though, should I go ahead and close this issue right away or wait for the PR to be merged?

[acinader]

it'll auto close when merged.