Re: Django Rest Framework Validation Order Issue

2020-07-01 Thread Özgür Akçalı
I think you also need to override *run_validators *method in your example 
project for it to work as you expect, just like you did for *to_internal_value. 
*If a validation error is raised fom run_validators method, *validate *method 
is not called, so you can not aggregate any possible errors raised from 
validation checks in that method.

Also, with this flow, you should take care not to raise validation errors 
in your custom *validate *methods one by one, but aggregate them and raise 
them or return them together there as well.

On Wednesday, July 1, 2020 at 1:45:00 PM UTC+3, Özgür Akçalı wrote:
>
> Though you can not achieve what you want by just overriding *validate, *and 
> not overriding *run_validation*. 
>
> This would be a major change in DRF though, as with the current behavior, 
> when you write your *validate *method, you can assume other validations 
> passed, so I imagine changing this behavior as default now would break a 
> lot of existing code. I think this would fit better as an optional 
> addition, maybe with a configuration option on existing serializer classes, 
> or with a different set of serializer classes.
>
> On Wednesday, July 1, 2020 at 1:59:33 AM UTC+3, Brent O'Connor wrote:
>>
>> I guess my point is that I don't feel like I should have to override 
>> anything and this should be DRF's default behavior. 🙂 Thank you for 
>> pointing out that `validation` should be the method that should be used. 
>>
>> On Tue, Jun 30, 2020, 5:42 PM Carl Nobile  wrote:
>>
>>> Brent, overriding the 'to_internal_value' method is normal and 
>>> encouraged, however, the 'run_validation' shouldn't generally be 
>>> overridden, what you can override is the 'validate' method which is called 
>>> by 'run_validation'.
>>> ~Carl
>>>
>>> On Tue, Jun 30, 2020 at 5:51 PM Brent O'Connor  
>>> wrote:
>>>
>>>> For anyone that might come across this thread, I went ahead and created an 
>>>> issue <https://github.com/encode/django-rest-framework/issues/7394> 
>>>> for this on Github.
>>>>
>>>> On Friday, June 26, 2020 at 5:08:03 PM UTC-5, Brent O'Connor wrote:
>>>>>
>>>>> I created an example project 
>>>>> <https://github.com/epicserve/drf-validation-issue> to illustrate 
>>>>> this issue, but basically if you have custom validation logic on a 
>>>>> serializer and the data has a field that isn't valid, then the errors 
>>>>> raised are only the field level errors and not the custom validation 
>>>>> logic 
>>>>> errors. This ends up being a bad user experience because the user can fix 
>>>>> the field error and then when post another request they can end up 
>>>>> getting 
>>>>> errors thrown in the custom validation. I'm thinking this might need to 
>>>>> be 
>>>>> an issue added the DRF project on Github, but thought I would check here 
>>>>> first. Please see the project example for more details.
>>>>>
>>>> -- 
>>>> You received this message because you are subscribed to the Google 
>>>> Groups "Django REST framework" group.
>>>> To unsubscribe from this group and stop receiving emails from it, send 
>>>> an email to django-rest-framework+unsubscr...@googlegroups.com.
>>>> To view this discussion on the web visit 
>>>> https://groups.google.com/d/msgid/django-rest-framework/ea8a9b5d-a671-42b4-9cee-043f28f8efa3o%40googlegroups.com
>>>>  
>>>> <https://groups.google.com/d/msgid/django-rest-framework/ea8a9b5d-a671-42b4-9cee-043f28f8efa3o%40googlegroups.com?utm_medium=email&utm_source=footer>
>>>> .
>>>>
>>>
>>>
>>> -- 
>>>
>>> ---
>>> Carl J. Nobile (Software Engineer)
>>> carl@gmail.com
>>>
>>> ---
>>>
>>> -- 
>>> You received this message because you are subscribed to the Google 
>>> Groups "Django REST framework" group.
>>> To unsubscribe from this group and stop receiving emails from it, send 
>>> an email to django-rest-framework+unsubscr...@googlegroups.com.
>>> To view this discussion on the web visit 
>>> https://groups.google.com/d/msgid/django-rest-framework/CAGQqDQL2UvWyjr6UgK%2BpCDexAc%2BC%2BeBUGxAwZYyaxC3dJYwGsQ%40mail.gmail.com
>>>  
>>> <https://groups.google.com/d/msgid/django-rest-framework/CAGQqDQL2UvWyjr6UgK%2BpCDexAc%2BC%2BeBUGxAwZYyaxC3dJYwGsQ%40mail.gmail.com?utm_medium=email&utm_source=footer>
>>> .
>>>
>>

-- 
You received this message because you are subscribed to the Google Groups 
"Django REST framework" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-rest-framework+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-rest-framework/86b2a1e3-21cf-466a-920c-4645b8125af4o%40googlegroups.com.


Re: Django Rest Framework Validation Order Issue

2020-07-01 Thread Özgür Akçalı
Though you can not achieve what you want by just overriding *validate, *and 
not overriding *run_validation*. 

This would be a major change in DRF though, as with the current behavior, 
when you write your *validate *method, you can assume other validations 
passed, so I imagine changing this behavior as default now would break a 
lot of existing code. I think this would fit better as an optional 
addition, maybe with a configuration option on existing serializer classes, 
or with a different set of serializer classes.

On Wednesday, July 1, 2020 at 1:59:33 AM UTC+3, Brent O'Connor wrote:
>
> I guess my point is that I don't feel like I should have to override 
> anything and this should be DRF's default behavior. 🙂 Thank you for 
> pointing out that `validation` should be the method that should be used. 
>
> On Tue, Jun 30, 2020, 5:42 PM Carl Nobile  > wrote:
>
>> Brent, overriding the 'to_internal_value' method is normal and 
>> encouraged, however, the 'run_validation' shouldn't generally be 
>> overridden, what you can override is the 'validate' method which is called 
>> by 'run_validation'.
>> ~Carl
>>
>> On Tue, Jun 30, 2020 at 5:51 PM Brent O'Connor > > wrote:
>>
>>> For anyone that might come across this thread, I went ahead and created an 
>>> issue  for 
>>> this on Github.
>>>
>>> On Friday, June 26, 2020 at 5:08:03 PM UTC-5, Brent O'Connor wrote:

 I created an example project 
  to illustrate this 
 issue, but basically if you have custom validation logic on a serializer 
 and the data has a field that isn't valid, then the errors raised are only 
 the field level errors and not the custom validation logic errors. This 
 ends up being a bad user experience because the user can fix the field 
 error and then when post another request they can end up getting errors 
 thrown in the custom validation. I'm thinking this might need to be an 
 issue added the DRF project on Github, but thought I would check here 
 first. Please see the project example for more details.

>>> -- 
>>> You received this message because you are subscribed to the Google 
>>> Groups "Django REST framework" group.
>>> To unsubscribe from this group and stop receiving emails from it, send 
>>> an email to django-rest-framework+unsubscr...@googlegroups.com 
>>> .
>>> To view this discussion on the web visit 
>>> https://groups.google.com/d/msgid/django-rest-framework/ea8a9b5d-a671-42b4-9cee-043f28f8efa3o%40googlegroups.com
>>>  
>>> 
>>> .
>>>
>>
>>
>> -- 
>>
>> ---
>> Carl J. Nobile (Software Engineer)
>> carl@gmail.com 
>>
>> ---
>>
>> -- 
>> You received this message because you are subscribed to the Google Groups 
>> "Django REST framework" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to django-rest-framework+unsubscr...@googlegroups.com 
>> .
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/django-rest-framework/CAGQqDQL2UvWyjr6UgK%2BpCDexAc%2BC%2BeBUGxAwZYyaxC3dJYwGsQ%40mail.gmail.com
>>  
>> 
>> .
>>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django REST framework" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-rest-framework+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-rest-framework/bfc354ad-f398-4380-a481-a1d1764f21f4o%40googlegroups.com.


Re: authorization after serialization

2020-06-24 Thread Özgür Akçalı
Example scenario: On a project, let's say we have the following models:

*User*: a user regisrered to the system
*Company*: a company operating on the system
*Role*: an entity that defines a set of permissions
*UserRole*: defines a role a user has on a company. A user's relation with 
a company is kept in this model. Has fields "user", "company" and "role", 
and basically means "*User A*" has permissions defined in "*Role A*" on 
"*Company 
A*"

Following is a sample json representing a user:

{
"name": "John Doe",
"email": "john...@example.com",
"roles": [
"role": {
"id": 10,
"name": "Administrator"
},
"company": {
"id": 100,
"name": "Example Company"
}
]
}


We have update action enabled on User APIs, so it is possible to send a *PUT 
*request to users endpoint and update a user's data, including their roles. 
One of the rules we have about updating a role is, if "User A" is trying to 
assign "Role A" to "User B" on "Company A", "User A" must have "Role A" on 
"Company A". This means that a user can not assign a role they don't have 
themselves. This check, as it is about authorization, I want to handle in a 
permission class and return a 403 error if fails. Following is a sample 
permission class for this:

class SamplePermissionClass(permissions.BasePermission):

def has_permission(self, request, view):
if view.action == 'update': 
   # Here we don't have the request validated, so we can't assume 
request structure is correct, request body has correct fields etc.
   # We also don't want to return a 403 response if request data is 
not valid, so we first verify that request data is valid, then perform 
permission checks if valid
   # and return False if they fail. In all other cases we return 
True
   user_roles = request.data.get('roles') or []
   for user_role in user_roles:
   role_data = user_role.get('role') or {}
   company_data = user_role.get('company') or {}
   if 'id' in company_data and 'id' in role_data:
   try:
   role = Role.objects.get(pk=role_data.get('id'))
   company = Company.objects.get(pk=company_data.get(
'id'))
   except (ObjectDoesNotExist, ValueError): # We don't know 
if id is a valid integer here. If it is None, or maybe a string, we may get 
a ValueError here
   pass 
   else:
   # Finally we can perform permission checks now
   if not user_has_permission(request.user, role, 
company):
   return False

return True
   


As you can see, to be able to perform our check, we needed to check that 
request data is valid for fields concerning this check. We have all these 
validation checks defined on the serializer, but nonetheless we need to 
perform those checks here as well to not raise an unexpected exception.

Alternatively, if we were sure the request data was valid before permission 
checks were run, the above code could be wirtten like this:

class SamplePermissionClass(permissions.BasePermission):

def has_permission(self, request, view):
if view.action == 'update': 
   user_roles = request.data.get('roles')
   for user_role in user_roles:
   role = Role.objects.get(pk=user_role.get('role').get('id'))
   company = Company.objects.get(pk=user_role.get('company').get
('id'))   
   if not user_has_permission(request.user, role, company):
   return False

return True



On Wednesday, June 24, 2020 at 4:28:18 AM UTC+3, 王祥 wrote:
>
> Can you give me some scenario about this. Paste some code here is better.
>
> 在 2020年6月21日星期日 UTC+8上午5:30:00,Özgür Akçalı写道:
>>
>> With default DRF request processing flow, auhtorization is run before 
>> serialization is done, that is, authentication_classes are processed first, 
>> and if they pass, validation checks and finally serialization are done. 
>> This is OK most of the time.
>>
>> But sometimes, I need to look into fields in request body to determine if 
>> request should be authorized, and return a 403 response if those checks 
>> fail. And when these checks involve nested serializers, I need to look into 
>> several levels deep fields in request jso

Re: authorization after serialization

2020-06-20 Thread Özgür Akçalı
Btw sorry, what I mean in first paragraph is permission_classes, not
authentication_classes

Le dim. 21 juin 2020 à 00:30, Özgür Akçalı  a
écrit :

> With default DRF request processing flow, auhtorization is run before
> serialization is done, that is, authentication_classes are processed first,
> and if they pass, validation checks and finally serialization are done.
> This is OK most of the time.
>
> But sometimes, I need to look into fields in request body to determine if
> request should be authorized, and return a 403 response if those checks
> fail. And when these checks involve nested serializers, I need to look into
> several levels deep fields in request json, and as the request is not
> validated at this step, I need to check that request json has the correct
> structrue first, before getting fields to use in permission checks. This
> often results in adding some of the validation checks in permission
> classes, and both complicates permission classes, and requires me to
> duplicate validation logic that is in serializers into permission classes.
>
> This led me into thinking if it would be better to be able to run
> validations first, and then perform permission checks. Do you think it
> would be a good idea? And possibly, is there any way of doing this with
> natural DRF request processing flow that I might have missed?
>
> --
> You received this message because you are subscribed to a topic in the
> Google Groups "Django REST framework" group.
> To unsubscribe from this topic, visit
> https://groups.google.com/d/topic/django-rest-framework/DgnWFCE-KXo/unsubscribe
> .
> To unsubscribe from this group and all its topics, send an email to
> django-rest-framework+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-rest-framework/8a13d55a-29a1-4dc3-95c1-d4f6f28dd1a0o%40googlegroups.com
> <https://groups.google.com/d/msgid/django-rest-framework/8a13d55a-29a1-4dc3-95c1-d4f6f28dd1a0o%40googlegroups.com?utm_medium=email&utm_source=footer>
> .
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django REST framework" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-rest-framework+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-rest-framework/CA%2Bt7k-XvF1x3oHXrZNkm3HtfP%3DLLrFp5XL-YQ-bz1Ge2R5MzMA%40mail.gmail.com.


authorization after serialization

2020-06-20 Thread Özgür Akçalı
With default DRF request processing flow, auhtorization is run before 
serialization is done, that is, authentication_classes are processed first, 
and if they pass, validation checks and finally serialization are done. 
This is OK most of the time.

But sometimes, I need to look into fields in request body to determine if 
request should be authorized, and return a 403 response if those checks 
fail. And when these checks involve nested serializers, I need to look into 
several levels deep fields in request json, and as the request is not 
validated at this step, I need to check that request json has the correct 
structrue first, before getting fields to use in permission checks. This 
often results in adding some of the validation checks in permission 
classes, and both complicates permission classes, and requires me to 
duplicate validation logic that is in serializers into permission classes. 

This led me into thinking if it would be better to be able to run 
validations first, and then perform permission checks. Do you think it 
would be a good idea? And possibly, is there any way of doing this with 
natural DRF request processing flow that I might have missed?

-- 
You received this message because you are subscribed to the Google Groups 
"Django REST framework" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-rest-framework+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-rest-framework/8a13d55a-29a1-4dc3-95c1-d4f6f28dd1a0o%40googlegroups.com.