Update other User object

[aBuder]

Hi,

I would like to talk about an general think or feature in parse server. At the moment I get an error 206 Cannot modify user xyz. I don't know why, I set an other user which is an admin in my parse server to acl but he cloud not update some data. Is there an solution to fix that without writing some custom CloudCode?

This is an generally concept :)

[georgesjamous]

As I understand it, Users are special class where for security only the user himself can update the user object.
I haven't tried what you did, but my suggestion is to go ahead a use a cloud function that does all the checking and then use MasterKey to update the target user's data.

Although, you should not need to do this since by architecture the user object should only hold login information (username, password and other stuff NOT related to user settings or personal info). Personal data and other related settings to a user should be stored in different classes.
If you do that, you can use Roles and ACL to modify these objects directly without proxying though a cloud code.

Hope this helps!

[aBuder]

Right but why user class then have ACL? The PR #4792 would use ACL and will solve the problem.

[flovilmart]

@aBuder while the Pr solves the problem, this is still a breaking behavioral change, which may not be acceptable for a majority of users. The PR is in review right now, commented upon.

[aBuder]

@flovimmart OK I hope that the change would be acceptet. I would also be an usecase that an Admin oder other user can change User Data. The previous behavior would also be possible with ACL. But at the momemt the ACL in user would not allow to edit other user.

[flovilmart]

I know what the changes do, but imagine a second, that one developer has bad ACL’s on all or some it’s users. And that now, with the proposed change, those users become ‘open’ and unprotected. What are you gonna say to those users when they suffer a leak, or exploitation of their data? That the PR that opened the floodgates of hell was ‘cool’ and solved ‘your problem’?

[aBuder]

@flovilmart Thats right, I understand that problem. It would be tricky

[aBuder]

@flovilmart When there would be an decision? If the PR would rejected, I have to look for an other solution as parse. I plan an solution for fitness centers and each fitness center has an own parse server instance. As an business case an manager of fitness center should modify the members and trainers. The members should connect with an mobile app to parse server.
In my case are 600 - 800 fitness centers and look for an solution without Cloud Code

[flovilmart]

If the PR would rejected, I have to look for an other solution as parse

I can't commit to a solution or another, but security is primordial, you would not want your 800 fitness centers being targeted by a security flaw right?

Also, now any users with a masterKey can modify the users and the ACL's. So you could technically do something in a cloud function that would do whatever you like.

look for an solution without Cloud Code

I'm not sure why you need not to have Cloud Code, perhaps parse is not the right tool for whatever you're trying to build.

[aBuder]

@flovilmart I would like to use normal POST, PUT, GET DELETE Operations on _User class and with the ACL also read and modify email. Also use normal query functions to fetch user with field email.

[flovilmart]

@aBuder I undestand what YOU want, that's not the issue, but you have to understand the consideration required to maintain this project safe.

[georgesjamous]

@aBuder we are working on a project right now that have similar grand architecture of yours.

We have centers that each run a single server instance (well, a cluster of them actually).
And in each of those have Directors, Managers, Employees ... and so on.
And each employee or manager or director have a different level of security clearance and responsibilities and actions that he can perform on other employees and so on...
We also actually have another server that serves as an Authority Granter (cross-center-access), we could have employees that can manage employees in another center.

If we were to directly allow members to edit members using _User ACL (actually that is not possible now without masterKey, but after the PR it will be), we would have to write additional functions and take additional security measures to prevent mistakes from happening.

That is why I am against the PR, because in general:
No user can EVER be allowed to edit other user information directly on _User, because this will put every other object or (_Role) that this user is part of at risk. If mistakes happen someone can reset someones's password and assume control of the user and all his privileges.

Our architecture is so complex, but should not be different than any system that implements different privileges for users and a group of users over others.

I recommend you start looking into _Role and SPLIT, SPLIT, SPLIT actions, privileges and other stuff into different classes, even the personal information should be in a totally different class that is protected by a _Role. (this makes sense and enhances Query speed using indexes)
That Role (named for example USERid_PERSONALINFOOBJECTid_Read /_Write) that control access to the user's info (this is just an example, but you get the idea)
That way there is no limit on functionalities and hierarchy, you can literally do anything!

Hope this helps with your decision :)

[aBuder]

@georgesjamous

That is why I am against the PR, because in general:
No user can EVER be allowed to edit other user information directly on _User, because this will put every other object or (_Role) that this user is part of at risk.

Why does an manager should not edit user information of an member in department, that is an normal use case. An IT admin should create new user and set or edit user information of other users. That is an normal and real world use case, and the use case already exists :)

I think the default behavior should not be hanged as the current behavior, but with an parameter in parse-server config like "modiyUserByACL". I could set if the ACL should be used, so that an user can edit other user, if the has the right ACL do do that. Otherwise the modify property in User should be obsolete now.

[georgesjamous]

@aBuder
ah, look where my concern is, at least the way I see it. (it may not be correct for everyone)

and yes

think the default behavior should not be hanged as the current behavior, but with an parameter in parse-server config like "modiyUserByACL". I could set if the ACL should be used, so that an user can edit other user, if the has the right ACL do do that.

providing an option like this could prove to be useful for some developers.
but,

in your example,

An IT admin should create new user ....

the IT can and should be able to edit the user's info Yes, but this should be done in other classes.
(another more direct explanation would be : classes that do not generate a session token at some point, classes that do not give the ultimate key for all the user's privileges at some point)
User related data like classes: UserInformation (name, nickname, phone, DOB), UserPrivileges, UserLoginActivity, UserVisitsActivity... can be edited by admins and secured by _Roles Yes.
This is how you edit the name, and other stuff in an IT admin scenario.

I see no reason why to put everything in _User class just because it is called user class.
_User class is owned by the User himself, it is the door that provide the user access to all his privileges and account once he signs in. I don't believe anyone should be able to edit it, not directly using ACL at least.

It is not wrong to do what you are trying to accomplish with _User; admins should be able to edit the name, email and date of birth of the user, Yes.

I think what we are actually debating is the architecture of the database itself :) not the editing of the user info per say.

All I am saying is that you can accomplish what you are trying to accomplish by a change in Database architecture. There is no need for _User class to follow the ACL.

[mullwaden]

As the author of the PR and reading the comments above I concur that it's probably not a good idea. However the docs should probably be updated :)

[flovilmart]

I will concur with @georgesjamous on this one. For the longest time, I've used the _user table solely for authentication purposes and locked down as much as possible. All public / private infos would go to external classes.

[flovilmart]

@aBuder can you try the master branch with your code? I just merged the fix for it. cc @mullwaden , @georgesjamous .

[aBuder]

@flovilmart I will check it today