Allow user to lockout with the master key

[Mardaneus86]

Fixes #3565 by allowing a user to remove the ACL of a User object completely when providing the master key. This way you could lockout specific users without removing their user account.

Not really sure about the unit test though. Since the master key is always used, I can't check if the user login actually fails. Is there a way to use Parse.User.logIn without the master key in the test suite?

[flovilmart]

Since the master key is always used

The masterKey should not be always used upon the login in that case. So that makes your trick probably broken for now and ineffective, as what you want to block is the ability to login, not really the ACL.

[Mardaneus86]

Not really sure what you mean. I was talking about the login calls in the unit tests, since Parse is initialised with a master key in the test suite. This makes the login call always succeed, even if the ACL is removed for the user. If there is an option to login without a master key in the test suite, the login would fail when the ACL is removed. Or am I missing something?

[flovilmart]

Yeah, the tests are not run all the times using the masterKey. As you did with your test, you have to explicitly pass the masterKey to use it.

The test you wrote is 'bad' in the sense that the login call should not succeed, therefore the fix you're proposing is incomplete

[Mardaneus86]

Oh, I thought it was using the masterkey all the time but apparently got confused. I explicitly logged the isMaster field in the DatabaseController find method to check that and it logged out a true value. But since that method is called without an acl in the UsersRouter login method, it of course is true. (and yeah, I know the unit test is 'bad', that's why I added the TODO 😄)

So my fix would effectively require a change in the UsersRouter like the expiry policy. It would need to check the ACL of the user that is returned from the database, and if it is not set return an error. But I'm wondering what error message should be returned in that case? Something like this:

throw new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, 'Your account is locked out.');

Also I'm wondering if I should only check for the specific user's ACL, or also for role and * ACL's?

This might be a little bit more complex than I initially anticipated. I'm hoping someone has a better alternative/suggestion? Maybe having a login hook that people would be able to implement for custom checks before allowing a user to login would be a more flexible solution?

[facebook-github-bot]

@Mardaneus86 I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

[Mardaneus86]

Closing this for the time being. I still think we would need a way to be able to block users without physically removing their account. Although the fix I tried to implement here, is not generic enough to suite all cases.

So until a fix is found I think the only solution you have, when you have this as a requirement, is to implement your own custom OAuth authentication provider. I still feel the login hook-option of my previous comment, would be the best generic way to go.

[flovilmart]

Maybe having a login hook that people would be able to implement for custom checks before allowing a user to login would be a more flexible solution?

That seems to be a reasonable solution, then you can keep a table with the locked-out accounts externally which makes it efficient.

[Mardaneus86]

Exactly, however I don't have the time to implement something like that right now. And also missing some in depth knowledge to implement it properly into parse-server I guess. Perhaps we could add this as an issue so someone else can take a stab at it?

[haroonKhan-10p]

@Mardaneus86 have you found a way to disable user login?

[Mardaneus86]

Unfortunately, no. I've switched to using a separate authentication provider in my project, as stated in one of the previous comments.

[pocketcolin]

If anyone else comes to this thread looking for a solution to disable user login, I was able to achieve this by changing the user's ACLs to only Admin R/W (Admin is a custom role I created). It looks like, as long as the user's ACLs prevent the user from reading their own user row in Parse, Parse will throw a 404 not found when they try to login.

NOTE: In order to remove a user's access from their own row's ACLs, you must use useMasterKey (I created a cloud function to handle the disabling of users). I believe this requirement is a result of this PR: #4860