From e3c1443a080a80b287c7d2b3baca902817d31d73 Mon Sep 17 00:00:00 2001 From: Scott Baker Date: Mon, 6 Oct 2014 22:58:48 -0700 Subject: [PATCH] re-did the fine grained permission checking stuff again --- planetstack/core/models/plcorebase.py | 17 +++-------- planetstack/core/models/user.py | 44 +++++++++++---------------- 2 files changed, 23 insertions(+), 38 deletions(-) diff --git a/planetstack/core/models/plcorebase.py b/planetstack/core/models/plcorebase.py index 2ad6f76..000e77e 100644 --- a/planetstack/core/models/plcorebase.py +++ b/planetstack/core/models/plcorebase.py @@ -76,14 +76,12 @@ class DiffModelMixIn: def changed_fields(self): return self.diff.keys() - @property def has_field_changed(self, field_name): return field_name in self.diff.keys() def get_field_diff(self, field_name): return self.diff.get(field_name, None) - class PlCoreBase(models.Model, DiffModelMixIn): objects = PlCoreBaseManager() deleted_objects = PlCoreBaseDeletionManager() @@ -113,12 +111,8 @@ class PlCoreBase(models.Model, DiffModelMixIn): return False if user.is_admin: return True - return False - def can_update_field(self, user, fieldName): - # Give us the opportunity to implement fine-grained permission checking. - # Default to True, and let can_update() permit or deny the whole object. - return True + return False def delete(self, *args, **kwds): # so we have something to give the observer @@ -153,11 +147,10 @@ class PlCoreBase(models.Model, DiffModelMixIn): def save_by_user(self, user, *args, **kwds): if not self.can_update(user): - raise PermissionDenied("You do not have permission to update %s objects" % self.__class__.__name__) - - for fieldName in self.changed_fields: - if not self.can_update_field(user, fieldName): - raise PermissionDenied("You do not have permission to update field %s in object %s" % (fieldName, self.__class__.__name__)) + if getattr(self, "_cant_update_fieldName", None) is not None: + raise PermissionDenied("You do not have permission to update field %s on object %s" % (self._cant_update_fieldName, self.__class__.__name__)) + else: + raise PermissionDenied("You do not have permission to update %s objects" % self.__class__.__name__) self.save(*args, **kwds) diff --git a/planetstack/core/models/user.py b/planetstack/core/models/user.py index 9b54da9..5a73ad7 100644 --- a/planetstack/core/models/user.py +++ b/planetstack/core/models/user.py @@ -100,6 +100,9 @@ class User(AbstractBaseUser, DiffModelMixIn): USERNAME_FIELD = 'email' REQUIRED_FIELDS = ['firstname', 'lastname'] + PI_FORBIDDEN_FIELDS = ["is_admin", "site", "is_staff"] + USER_FORBIDDEN_FIELDS = ["is_admin", "is_active", "site", "is_staff", "is_readonly"] + def __init__(self, *args, **kwargs): super(User, self).__init__(*args, **kwargs) self._initial = self._dict # for DiffModelMixIn @@ -200,38 +203,28 @@ class User(AbstractBaseUser, DiffModelMixIn): msg.attach_alternative(html_content, "text/html") msg.send() - def can_update_field(self, user, fieldName): - from core.models import SitePrivilege - if (user.is_admin): - # admin can update anything - return True - - # fields that a site PI can update - if fieldName in ["is_active", "is_readonly"]: - site_privs = SitePrivilege.objects.filter(user=user, site=self.site) - for site_priv in site_privs: - if site_priv.role.role == 'pi': - return True - - # fields that a user cannot update in his/her own record - if fieldName in ["is_admin", "is_active", "site", "is_staff", "is_readonly"]: - return False - - return True - def can_update(self, user): from core.models import SitePrivilege + _cant_update_fieldName = None if user.is_readonly: return False if user.is_admin: return True - if (user.id == self.id): - return True # site pis can update site_privs = SitePrivilege.objects.filter(user=user, site=self.site) for site_priv in site_privs: if site_priv.role.role == 'pi': + for fieldName in self.diff.keys(): + if fieldName in self.PI_FORBIDDEN_FIELDS: + _cant_update_fieldName = fieldName + return False return True + if (user.id == self.id): + for fieldName in self.diff.keys(): + if fieldName in self.USER_FORBIDDEN_FIELDS: + _cant_update_fieldName = fieldName + return False + return True return False @@ -252,11 +245,10 @@ class User(AbstractBaseUser, DiffModelMixIn): def save_by_user(self, user, *args, **kwds): if not self.can_update(user): - raise PermissionDenied("You do not have permission to update %s objects" % self.__class__.__name__) - - for fieldName in self.changed_fields: - if not self.can_update_field(user, fieldName): - raise PermissionDenied("You do not have permission to update field %s in object %s" % (fieldName, self.__class__.__name__)) + if getattr(self, "_cant_update_fieldName", None) is not None: + raise PermissionDenied("You do not have permission to update field %s on object %s" % (self._cant_update_fieldName, self.__class__.__name__)) + else: + raise PermissionDenied("You do not have permission to update %s objects" % self.__class__.__name__) self.save(*args, **kwds) -- 2.43.0