Deployment or bust, part 3: Permissions
Continuing from where we left off, we’re going to enforce stricter permissions and give every user their own private set of database objects to work with. Right now we just have the humble TaskList object, so every TaskList is going to have an owner, every TaskList is going to be only viewable/modifiable by its owner, and we’re going to lock down the relevant TaskList views so that there’s no app-critical endpoint that an anonymous user can hit.
Changing the TaskList model
We’re going to add a User foreign key to the TaskList model that represents that TaskList’s owner/creator (see next section). The Django migrations tool needs us to specify a default owner for all the currently-existing TaskLists, but that’s kind of annoying to do, so what I actually did was:
- Allow the foreign key field to be nullable (i.e. allow TaskLists to have null owners)
- Migrate
- Give all the TaskLists with null owners a default owner in the admin panel
- Set the field to be required (i.e. may not be null)
- Migrate, ignoring the migration warning (Django lets us do this, and we know it’s OK because we just made sure no TaskList has a null owner)
- There is no step 6.
Could also just not have bothered since we don’t have any TaskLists we care enough about not to delete. Eh.
Permissions model
Django has a built-in permissions model, but I’m avoiding it for now, because the current model I want to achieve is simply:
A user may only view, modify or delete task lists that they themselves have created.
So for every view that isn’t “creating a task list”, we check that the user
making the request matches the task list’s creator, which we set on task list
creation. This isn’t too bad, although there is the consideration about whether
or not we ever leak the existence of another user’s tasks. I found this good
blog post about checking ownership through the
UserPassesTestMixin
(or more specifically, about not doing that) that gave me guidance on how to
restrict the class-based DetailView
for my task lists to only work for
logged-in users who own the task list.
Anyway, this is fairly straightforward, just add a bunch of filters to all the
QuerySets
in all the views that restrict objects to ones where
creator=request.user
.
…damn, I just realised that I’m probably going to have to start writing proper
tests that call the backend API directly, because I’m not going to be able to
confirm that a user can’t delete another user’s task lists via the
deletetasklists
call, say, just by using the frontend. I guess I should be
writing tests anyway.
Custom user problems again and again forever
Trying to create another user to at least test that users have their own private views and I run into a slight issue. Remember this code?
class UserAdmin(admin.ModelAdmin):
# i'll get around to this later
pass
Yeah, this isn’t going to work, because we need an admin-friendly way of setting the password hash from a password. Just using the default ModelAdmin setup means we have to create the password hash ourselves and paste it into the field in the admin page for the user, which I don’t really want to have to do.
Anyway, after cross-referencing their custom users admin
example
(you have to override the custom add_fieldset
attribute, and I still don’t
know what "classes": ["wide"]
means, but whatever) with the UserAdmin
source and editing admin.py
, I can finally add regular users to the system
via the admin site. I can then log in as a newly created user and confirm that
I can’t see other users’ task lists, and make task lists of my own, which are
then not viewable by other users. Great success!
URL IDs and enumeration attacks
Since we’ve apparently committed to leaking as little information about even the existence of other users’ task lists as possible, we should probably stop using auto-incrementing integer IDs for the task lists in our URLs. (If I create a task list and it gets assigned id 56, and then I create another task list a while later and get assigned id 61, I know 4 task lists were created by other users in the meantime and that they were assigned ids 57-60, which would be the first step to trying to crack into them.) I’m thinking we use UUIDs instead, possibly converting to Base64 for use in URLs.
We can either add a UUID as an extra column to the task list table, or change out the primary keys to be UUIDs themselves. The latter sounds annoying to try to do, but we don’t have any data we care about yet so we can just drop the whole database again in the worst case.
Actually, we might be chilling using the UUID as the new primary key, since the docs give an example of doing exactly that.
…time passes…
OK, I trashed the database a bit, but now UUIDs work. Transforming them into
Base64 for use in URLs would make things complicated and probably isn’t worth
it, so we just have UUIDs in the URLs now, e.g.
/app/tasks/4c8e5753-0e6f-44c6-9c41-5ed1bf90efd1
. Just rolls off the tongue,
doesn’t it?
Upshot
Incredibly, I think everything that I deemed mission-critical for deployment is out of the way now, so next time we’re going to go through the Django deployment preflight checklist and deploy to my VPS. w00t!
(continued in Part 4)