Fix/1089/email enumeration: Return generic response #1114
Conversation
|
I tend to think that adding some level of back-off delay is a better solution here? Can we look at throttling perhaps? We're already using it on the login endpoint for a similar purpose. Check out the code here. I'm happy to include this generic error message if that's what the security testers want but I think it's kind of pointless and that rate limiting is the better solution. |
These are two separate things really:
agree we should do both. Maybe 2 should be a separate PR |
|
I'd argue that (2) should also be in the branch named 'email-enumeration' and since it's likely only ~10 lines of code, let's just do it? |
|
Sounds good |
api/custom_auth/views.py
Outdated
|
|
||
|
|
||
| class ThrottledUserViewSet(UserViewSet): | ||
| throttle_classes = [ScopedRateThrottle] |
There was a problem hiding this comment.
This will set the throttle for all the endpoints in this viewset whereas I think we only really want it on signup, right? Should we just override the get_throttles method instead?
| @@ -20,6 +23,7 @@ | |||
| CustomAuthTokenLoginWithMFACode.as_view(), | |||
| name="mfa-authtoken-login-code", | |||
| ), | |||
| path("", include(throttled_user_router.urls)), | |||
| path("", include("djoser.urls")), | |||
There was a problem hiding this comment.
Looking at what we actually use djoser for, I think we could get away with replacing this with just the overridden viewset instead, right? Maybe that's not a good idea just incase anyone is using the additional endpoints in their open source installation but maybe we could add a comment to remove them in the next major release?
There was a problem hiding this comment.
Yeah, that's a good point. I will add the comment
No description provided.