Rework and clean up user management endpoints

[peterthomassen]

No description provided.

[peterthomassen]

Deletion and keeping domains is not an option; at the point when the account is deleted, no domains can exist.

@nils-wisiol How do we want to handle this? We can either delete the user's domains automatically when deletion is requested, or we can reject the request if there are domains so that the user has to delete domains beforehand. I'm leaning slightly towards the second option. What do you think?

[peterthomassen]

@nils-wisiol

Also, let's discuss how to arrange our user management views in the context of user deletion. Until now, we use djoser's UserView which is a RetrieveUpdateAPIView and does not have a delete() method, so deletion is currently not possible. I see several ways to change this:

• djoser has a separate UserDeleteView which allows deletion upon POST That's not very RESTful, though.
• We can extend the UserView so that one can DELETE on the auth/me/ endpoint. However, we then have to copy some code from djoser, which seems not ideal in terms of maintainability.
• The general solution would be using djoser's UserViewSet which comes with all sorts of other functionality like activating accounts, changing username (= email in our case), ... Some of that would also be interesting for us, but probably not all. The thing is quite configurable though, and unwanted functionality is easily turned off. But precisely because of that flexibility, this option introduces a significant amount of complexity behind the scenes (i.e. in the classes we have to import), and I'm not sure we want to have that in such a crucial part of our platform.

We should discuss this in the broader context, also having aspects of user locking etc. in mind.

[nils-wisiol]

We can either delete the user's domains automatically when deletion is requested, or we can reject the request if there are domains so that the user has to delete domains beforehand.

Yes, I believe asking the user to delete her domains first is reasonable. Otherwise, misguided requests may be fatal. In context of GDPR, we need to determine if we store personal data at all and how to proceed with backups that we may have. However, this is an operational issue.

Also, let's discuss how to arrange our user management views in the context of user deletion.

Before discussing any implementation, let's list the use cases for our user management here:

1. Create new users, possibly limited by invitations(1)
2. Unlock account (or any other abuse limitation)
3. Verify email address(1)
4. Second factor authentication(1)
5. Update email address
6. Change password
7. Recover passwords
8. Token management
9. Delete user

(1) we're not doing this right now, but any solution should have an option to enforce this

Please update as you seem fit. Once we're happy with the list, let's move the discussion to djoser and implementation.

[peterthomassen]

A few reasons to not continue using Djoser:

• In March 2019, a vulnerability /users/me/ endpoint allows POST requests sunscrapers/djoser#366 was reported (unauthorized creation of users, and according to changelog of the fix also with escalation to editing other users), but there was no reaction for more than two weeks. The report was very clear ("POST /users/me/ bypasses validation"), but the response after two weeks suggested to update to the newest version, and asking for more details, instead of looking into the problem. (The issue was fixed later that day.)
• Comprehensive functionality is provided by UserViewSet which is included in the default routes, but it is completely undocumented.
• This UserViewSet uses deprecated DRF functionality, djoser deprecation warning sunscrapers/djoser#370
• Further, UserViewSet has dead code regarding user deletion (https://github.com/sunscrapers/djoser/blob/master/djoser/views.py#L341 refers to an action called remove, but it does not exist in DRF)
• The documentation is generally a bit unclear. It claims, for example, that the response of a user deletion request contains the current password.

I am going to check out some alternatives.

[peterthomassen]

Putting this on hold until #180 is merged.

[nils-wisiol]

Let's move away from djoser asap 🤐

[peterthomassen]

django-rest-registration (user management) in combination with django-rest-knox (token management) supports all requirements, except:

• delete user. This is trivial to implement ourselves. I suppose it is not implemented upstream because deletion often needs to trigger side effects (like domain deletion), which upstream cannot foresee.
• 2FA. However, django-rest-registration has an issue for it (Research 2-step authentication apragacz/django-rest-registration#53).

django-rest-registration is rather new, but is really well-maintained, developed at high pace, and with solid design and code quality. I think it's currently the best solution.

Other solutions evaluated: django-rest-auth, django-allauth (these have weird cross-dependecies), djoser, manual impementation

[peterthomassen]

(django-rest-knox is recommended by DRF if you wanna go beyond very basic-only TokenAuthentication.)

[My1]

How do we want to handle this? We can either delete the user's domains automatically when deletion is requested, or we can reject the request if there are domains so that the user has to delete domains beforehand. I'm leaning slightly towards the second option. What do you think?

sorry if I chime in but I want to also fully agree.

knocking down your account can be a really annoying problem, as you have to restore all those records and domains (and not to forget, re-set up DNSSec), in my opinion, there may even be an argument for making deleting your domains a little bit harder.

That is what I done in an implementation I have, with basically the domain list containing timed delete buttons, which when clicked enact a timed confirmation prompt (including extra checks which invalidate everything if the target domain has been changed in any way), so accidential deletion of domains isnt easy, although obviously 3 requests, list, delete, confirm is obviously overkill for an API, but something to not shut yourself down too easily would probably be not too bad.

depending on your domain, especially on the more traditional ones the DS Lifetime is easily a day, which makes a recovery from an accidentially removed domain need time, and then we shouldnt forget that some registrars have no direct avenue for updating DS Records on their site, but support is needed, which just adds more.

on the other hand forcing a user to kill all records of a domain before being able to delete it can be REALLY tedious.

one probably idea that could be interesting (and crazy, I know) could be that upon deletion of an account (with or without extra confirmation doesnt matter) the domains could be immediately un-published and account (including tokens) disabled and an email gets sent to the account holder allowing him/her to re-activate the account if so desired within a certain timeframe.

for accounts disabled that way all requests pointing towards and/or using that account/its data should ideally fire up some error instead of just using the "locked" state, to REALLY throw signals to any monitoring or similar systems that something may be wrong, 423 Locked sounds interesting for that.

[peterthomassen]

Thanks for your input!

We were discussing whether account deletion should require deletion of all domains. We were not discussing whether domain deletion should be allowed if the domain has records.

It has long been settled that deleting a domain always works (including when records are present, which will then be deleted). We decided that asking for confirmation is a UI issue.

Regarding account deletion, we have agreed that users should first delete their domains (which is not going to be a lot of work, so it's acceptable, and prevents fatal errors from happening).

[peterthomassen]

implemented with #222