search

iplweb / django-password-policies-iplweb

Django unicode-aware password policies.
Other
7 stars 21 forks source link

added safety checks to context processor #11

Closed kostrom closed 3 years ago

kostrom commented 3 years ago

I added some safety checks to prevent this context processor from raising an unnecessary exception. rest_framework can set user = None django.contrib.auth.context_processors.auth is okay with that as long as an attribute exists

kostrom commented 3 years ago

Sorry for the late reply. I was on vacation. I can do that.

kostrom commented 3 years ago

My reproduction involves a REST_FRAMEWORK setting, but I don't want to add that to the requirements to run a test. I'm trying to mock out some of that, but might not get it today.

REST_FRAMEWORK['UNAUTHENTICATED_USER'] = None

kostrom commented 3 years ago

Sorry, I'm not able to get a working request with a middleware override, that also recreates these same conditions.

My fix is essentially just a basic "Check for None before you use the object."

The use case is admittedly not good practice, and I may have corrected the underlying problem that led to this, but if someone has those problems, you want them to run into problems in their code, not in your middleware.

mpasternak commented 3 years ago

Ok. I reviewed the change and I see where's the problem. What is exact configuration you're running this on (if that's not a secret, of course)? From what I understand, your REST framework doesn't set "user" attribute on the "request".

On the other hand, why would you want to use django-password-policies with REST framework views? I guess your REST framework has it's own authentication mechanisms... my question is because I believe django-password-policies is somewhat an "interactive" package, while REST API is... an API. If I'd do that, my REST API would return some 4xx error with "Password Expired" message or something like that.

Let's discuss.

kostrom commented 3 years ago

There's an interactive page, and there's an api. I actually redid the request from scratch because I forgot about this for a while -- sorry about that. And yes, the specific configuration which led to the problem was itself an error -- but password policies shouldn't mask it by raising its own exceptions when we can easily prevent that. (closing this one)