From 2fc070e6c5e41c12f03882a0426a11f0983f76fb Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Fri, 9 Jun 2023 14:17:55 -0500 Subject: [PATCH] Review feedback: updated developer documentation --- docs/developer/user-permissions.md | 22 ++++++++++++++++------ src/zap.conf | 4 ++++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/docs/developer/user-permissions.md b/docs/developer/user-permissions.md index 281a58c1b..af5aa1259 100644 --- a/docs/developer/user-permissions.md +++ b/docs/developer/user-permissions.md @@ -22,12 +22,22 @@ role or set of permissions that they have. We use a `UserDomainRole` ## Permission decorator The Django objects that need to be permission controlled are various views. -For that purpose, we add a very simple permission mixin -[`DomainPermission`](../../src/registrar/views/utility/mixins.py) that can be -added to a view to require that (a) there is a logged-in user and (b) that the -logged in user has a role that permits access to that view. This mixin is the -place where the details of the permissions are enforced. It can allow a view -to load, or deny access with various status codes, e.g. "403 Forbidden". +For that purpose, we have a View subclass to enforce user permissions on a +domain called +[`DomainPermissionView`](../../src/registrar/views/utility/permission_views.py) +that can be added to a view to require that (a) there is a logged-in user and +(b) that the logged in user has a role that permits access to that view. This +mixin is the place where the details of the permissions are enforced. It can +allow a view to load, or deny access with various status codes, e.g. "403 +Forbidden". + +In addition, we now require all of our application views to have a logged-in +user by using a Django middleware that makes every request "login required". +This is slightly belt-and-suspenders because our permissions view also checks +that the request includes a logged in user, but it avoids accidentally creating +content that is publicly available by accident. We can specifically mark a view +as "not login required" if we do need to have publicly accessible content (such +as health checks used by our platform). ## Adding roles diff --git a/src/zap.conf b/src/zap.conf index b2234250e..e5e7b4d04 100644 --- a/src/zap.conf +++ b/src/zap.conf @@ -87,6 +87,10 @@ 10062 FAIL (PII Disclosure - Passive/beta) 10095 FAIL (Backup File Disclosure - Active/beta) 10096 FAIL (Timestamp Disclosure - Passive/release) +# Our sortable table of domains uses timestamps as sort keys so this appears as +# a false-positive to the OWASP scanner +10096 OUTOFSCOPE http://app:8080 +10096 OUTOFSCOPE http://app:8080/ 10097 FAIL (Hash Disclosure - Passive/beta) 10098 FAIL (Cross-Domain Misconfiguration - Passive/release) 10104 FAIL (User Agent Fuzzer - Active/beta)