keeping Personal Info personal....

[acinader]

When doing queries on the user object, the response contains all fields. So if your user table has email addresses in it, I can pick your app id out of your ios app and the hit the rest endpoint and get all of your users email addresses. I'd like to be able fix this for our app, and maybe I should fix it for parse-server in general?

The basic idea is: if your not querying your own record or not using the master key, then don't return fields in the user table that are "sensitive". Simple enough?

I don't think that I can use any existing functionality to restrict which fields are returned. So what I did was make a beforeFind hook and then select just the 'whitelist' fields. That works fine.

So the problem that remains is for get requests (i.e. an id is provided) instead of find requests.

I'm game to submit a pr to apply the beforeFind hook to the get which seems like the right thing to me, OR i could add a beforeGet hook if that's what folks thinks would be better.

OR better yet, I am just missing something obvious :).

[flovilmart]

I think I recall the email addresses we removed when querying on parse.com, I still have valid accounts there and I could definitely look for it.

[acinader]

@flovilmart well, in our case, we've added more than just email that is sensitive (zip, phone which are both legally PII), so it would seem that having something general that could even be used for other classes would be useful. For me, I'm not the interested in trying to get field level permissions at this point, but at least being able to do it with hooks....in which case I need to get my hands on get requests :).

[acinader]

but yes, if this was done on parse.com, it would be good for us to strip the email address when not a master or self query of the user table, which I'd be glad to wrap into this effort (though prolly two sep pr as it wouldn't use the same method.....).

[flovilmart]

yep, stripping email on user responses should be done!

[acinader]

on it.

[FridaySG]

Yeah, this can be a pretty nasty problem for many reasons. Would be nice if we could build more security into Parse now that it's open-sourced. Like some sort of mechanism that only allows users to connect from specific client applications and things of that sort.

Returning to the original issue though, I crossed this bridge by creating a "PersonalInfo" class and setting the ACL's read/write restricted to the user in which they belong. Any sensitive data is saved in there.

[flovilmart]

That'a usually what i'd recommend. Use _User for login and publicly available info, or lock it down it your app don't need it. Use other objects for private info.

[mbxt]

I appreciate the concern for security, but although this is a small change, it introduces a breaking change. Tagging it in release "2.3.0" is inconsistent with npm semantic versioning, unless I'm mistaken. (https://docs.npmjs.com/getting-started/semantic-versioning). Is it the intent of this package to disregard a versioning scheme that many rely on?

[flovilmart]

There are other changes and refactorings that made the 2.3.0. It doesn't break the REST API, so we didn't tag it 3.0.0.
Also, the email key should have been probably hidden to match parse.com behavior.

If you are concerned about the release cycle and tagging, you can very well chip in and contribute yourself.

As a rule of thumb:

Patches: bug fixes, non breaking internal changes
Minor: breaking internal changes (config etc...)
Major: breaking REST API changes.

You should probably semver for automatic update on patches (~> 2.2.0) or like we do, fix all dependencies.

[acinader]

@mebmichael just to chime in since this was my change.

a) I was blissfully unaware, really, of the semvar rules despite having read them, so thanks for pointing it out now that I have some context that is relevant to help me understand the implications.

b) with regards to this change, it would probably be more accurate to call this a regression than a breaking change. Parse.com did not leak pii. I discovered the issue when i was doing some testing and was moderately panicked as I am aware that leaking pii has the potential to expose companies legally. I had a fix merged in by @flovilmart within 36 hours of my discovery.

If we weren't in the process of packaging up 2.3 at the time, I would have advocated for releasing a patch (which still might arguably be the right thing to do now so the fix is more widely distributed.)

Finally, I'd also argue that any functionality that depends on making people's email public without their knowledge should be broken and is probably illegal in at least some jurisdictions (like the EU).

Maybe none of that impacts how it should be versioned in your mind, and I'd be open to hearing your thoughts on it.

So its after the fact at this point, but knowing what I was thinking, what would you suggest would have been the right course of action?

Best and thanks again for a thought provoking question.

[mbxt]

@flovilmart I agree that this change should definitely have been made ASAP, though I would argue that for some client APIs, removal of a field is a breaking change (i.e., JavaScript). Were you ever able to determine if parse.com excludes the email field? Anyway, thanks for the response and the info :)

@acinader Thanks so much for providing the fix (and for the thoughtful response)! That's hugely appreciated. Yes, I was mostly wondering why things suddenly disappeared in my app when only a minor version changed, and it took a little digging to get to the answer (and more digging after that to get to the reason behind the answer). However, I'm kind of glad that it happened because I'm now aware of the issue you brought up, though I could have easily missed that small detail. I suppose if it's seen more as a patch than a feature, that makes much more sense. Anyway, thanks again for the response!

[acinader]

I can confirm that parse.com does not leak email from the user rest request. my change brought us into compliance with the parse.com behavior.