From 6271633432d9c6e2f60c694a3d301849b2dcb5c4 Mon Sep 17 00:00:00 2001 From: Tobias Bengfort Date: Wed, 29 Sep 2021 20:09:20 +0200 Subject: [PATCH] fix StudyMixin: avoid leaking study existance in API views A typical view can respond with 403 when a users is not authenticated or does not have the necessary permissions and with 404 if the requested object does not exist. It is important to get the order of those checks right. Responding with 404 too early can leak the existance of objects. Studies are special because we need them in order to check permissions in study context. So the correct order is: 1. authentication 2. get study 3. check permissions 4. get other objects The order of (2)-(4) works fine. (1) is also guarenteed to be first thanks to django-stronghold. However, for API views we disable django-stronghold and use custom authentication. That custom authentication is run during `dispatch()` while the study is fetched in `setup()`, so this is actually the wrong way around. I have some ideas how to implement this: - in dispatch - guaranteed to run (and potentially raise 404) - order of checks depends on order of mixins - StudyMixin must be in between APIAuthMixin und PermissionRequiredMixin - as a cached property - not guaranteed to run - order of checks depends on first use of self.study (probably correct, but no guarantees) - order of mixins doesn't matter - hook into PermissionRequiredMixin methods - guaranteed to run (as long as PermissionRequiredMixin is used) - run immediately before permission checks - order of mixins doesn't matter --- castellum/studies/mixins.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/castellum/studies/mixins.py b/castellum/studies/mixins.py index 5a016582c..591f5d384 100644 --- a/castellum/studies/mixins.py +++ b/castellum/studies/mixins.py @@ -57,9 +57,9 @@ class StudyMixin: key = 'study_pk' if 'study_pk' in self.kwargs else 'pk' return get_object_or_404(qs, pk=self.kwargs.get(key)) - def setup(self, request, *args, **kwargs): - super().setup(request, *args, **kwargs) + def has_permission(self): self.study = self.get_study() + return super().has_permission() def get_permission_object(self): return self.study -- GitLab