[#912] Harden access control on case-details/documents #329
Conversation
Bartvaderkin
force-pushed
the
feature/912-harden-cases-access
branch
from
00b1064
to
68a1a9f
Compare
Bartvaderkin
changed the title
|
I'll rebase this once the other changes land and remark for review as Anna hasn't looked at this yet. |
Bartvaderkin
force-pushed
the
feature/912-harden-cases-access
branch
from
68a1a9f
to
4db3f1d
Compare
| @@ -153,6 +155,10 @@ def get_context_data(self, **kwargs): | |||
| case = fetch_single_case(case_uuid) | |||
|
|
|||
| if case: | |||
| # check if we have a role in this case | |||
| if not fetch_roles_for_case_and_bsn(case.url, self.request.user.bsn): | |||
| raise PermissionDenied() | |||
There was a problem hiding this comment.
TBH checking permissions in get_context_data seems odd and scattering permission checks between test_func and get_context_data makes it hart to understand the authorization logic.
I'd suggest putting all permissions in one place (test_func or get or dispatch)
Since we already use mixins for permissions and this code is also used in CaseDocumentDownloadView it can also be put in the separate mixin
| @@ -252,12 +259,27 @@ def handle_no_permission(self): | |||
| return super().handle_no_permission() | |||
|
|
|||
| def get(self, *args, **kwargs): | |||
| info_object_uuid = kwargs["object_id"] | |||
| case_uuid = kwargs["object_id"] | |||
There was a problem hiding this comment.
I'd also suggest to put test_func and permission checks in get in one place
Bartvaderkin
force-pushed
the
feature/912-harden-cases-access
branch
from
63cae42
to
ed398d5
Compare
|
@annashamray I made an attempt to your suggestion to centralize the access checks: I dumped the regular mixins and turned them into a dispatch()-override because of the order in which the mixins super().dispatch() methods would execute (and we want auth/bsn check before we grab data). Feel free to merge/append/refactor if this holds up the case confidentiality tickets |
Continues from PR #327 & taiga 890