Permissions integration

[dc2007git]

Overview

This PR will integrate user permissions into different views. Taking advantage of our class-based view system, we can utilise the PermissionRequiredMixin which enables us to load a view only once a user's specific permission (which is specified in that very view) is checked. Else, return (at the moment) 403.

n.b. if you wish to test these permissions, you must delete all containers, images AND volumes on your device and rebuild from there for them to be integrated.

Code changes

create_groups.py - assign user permissions to each group that exists in NPDA. I have removed the old system of permissions (which we use in E12), where we have a set of permissions for 'viewing' all models, 'creating' all models etc., and then assigning each user group access. The approach implemented in this PR is more specific and explicit, allowing for simple permissions customisation in the future if we wish to do so, but at the expense of more lines of code.

mixins.py - slightly increase verbosity of logging when a user enters a view, showing their role and superuser status

views/patient.py - implement permissions checks

views/visit.py - implement permissions checks

settings.py - remove duplicated ALLOWED_HOSTS

mixins.py - add CheckPDUMixin for list views.

Documentation changes (done or required as a result of this PR)

Required - update permissions in docs to specify NPDAUser-related perms

Related Issues

13

[dc2007git]

@eatyourpeas can I ask for your review on this? Specifically the CheckPDUMixin in mixins.py and its functionality - remember we were trying to stop bad actors from changing session variables to try and access PDUs that they shouldn't be able to.

Also, before we merge I'm just waiting for Amani to get back to me about the new user groups - Coordinator, Editor, and Reader. If she says so, then before we merge this PR we'll have to implement those groups instead of using Lead Clinician, Clinician etc. as we have.

So just really requesting a general functionality review please!

[eatyourpeas]

Great start @dc2007git - so far two things:

1. Here I have created a user jane.admin@nhs.net with coordinator permissions at Addenbrookes. She is able to update her own record to make herself superuser/rcpch_audit_team_member and so on.

   

2. If I then update the url to the unique id for me I am able to edit a different record - I am registered at King's so this should not be possible.

   

That must mean that the mixin is letting me through. Have a look and we can try and troubleshoot together if no dice.

[dc2007git]

Thanks for the review @eatyourpeas . Great points, thanks for picking up on them!

1. Users definitely should not be able to change their own records to make themselves superusers. I have recreated a user of type Coordinator, and have been able to reproduce this. Coordinators DO have the permission set to change_npdauser, which means they can technically access their own profile - and so possibly what we need is to only render the is_superuser / is_admin checkbox for current superusers. Is it also a good idea to stop those with the change_npdauser permission from changing their OWN information (ie they have to contact the rcpch team)?

2. For the Update view, the mixin isn't in place! So should be an easy case of just adding it, looking something like this: check if npdauser1 with pdu1 is either: in the same pdu as npdauser2 OR superuser OR rcpch audit team.

Let me know if you have any other thoughts :)

[dc2007git]

@eatyourpeas another thought - we should perpetuate this idea of users with add/change_npdauser permissions only being able to add/change users to permissions equal to or lower than theirs. So a Coordinator should not be able to add an RCPCH Audit Team member, for example.