rsinger86
commented
2 years ago This looks good, thank you for pulling it together. There are two parts to my response (a) some questions about this pull and (b) and I wanted to pick your brain about taking an altogether different approach to field-level permissions. If you agree with (b) then you can ignore (a).
A)
- Why not pass the condition methods as strings? (the statements are no longer JSON serializable if they contain Python vars)
- Why only evaluate the conditions if the instance is available on the serializer?
(B)
Lately, I've started to think it may be better to take a procedural approach with field permissions. See pseudo code below.
The access policy class could define a scope_fields method like this:
class AccountAccessPolicy(AccessPolicy)
@classmethod
def scope_fields(cls, request, fields: dict, instance=None):
return fieldsThen the serializer mixin could be simplified:
class FieldsAccessMixin(object):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self._apply_fields_access()
@property
def access_policy(self) -> AccessPolicy:
meta = getattr(self, "Meta", None)
if not meta:
raise Exception("Must set access_policy inside Meta for FieldAccessMixin")
access_policy = getattr(meta, "access_policy", None)
if not access_policy:
raise Exception("Must set access_policy inside Meta for FieldAccessMixin")
return access_policy
@property
def request(self) -> Request:
request = self.serializer_context.get("request")
if not request:
raise Exception("Must pass context with request to FieldAccessMixin")
return request
def _apply_fields_access(self):
request = self._get_request()
policy = self._get_access_policy()
self.fields = policy.scope_fields(request, self.fields, instance=self.instance)
gianpieropa
Now we can specify conditions and actions for setting read_only fields. The conditions will be evaluated only if the instance is available (so only for patch, put). To differentiate between conditions on statements and conditions to set read_only fields, the methods will be passed not as string but directly as methods and will have as input the instance. I implemented this code to satisfy the necessity to centralize the permissions so i created this pull request. The next step would be to have the ability to access request and view, but I'm not sure of the usefulness