jpadilla / django-rest-framework-jwt

JSON Web Token Authentication support for Django REST Framework
http://jpadilla.github.io/django-rest-framework-jwt/
MIT License
3.19k stars 649 forks source link

Whats the rationale for deprecating get_user_id_from_payload? #463

Closed fergyfresh closed 5 years ago

fergyfresh commented 5 years ago

Why would this not be the default way for getting the instance of the user from the JWT? https://github.com/GetBlimp/django-rest-framework-jwt/blob/master/rest_framework_jwt/utils.py#L70

Otherwise I would need to add an extra index on username that I currently do not have? Doesn't really matter to me as I can look under the hood at the function and use the code that exists in the base function `payload.get('user_id'), but why remove the function?

Alex3917 commented 5 years ago

The main argument for using the username instead of having the primary key of the user in the payload is that the latter lets anyone who signs up for an account know how many registered users you have and what your growth rate is.

fergyfresh commented 5 years ago

That's a backend only function though...

On Wed, Mar 6, 2019, 7:33 PM Alex Krupp notifications@github.com wrote:

The main argument for using the username instead of having the primary key of the user in the payload is that the latter lets anyone who signs up for an account know how many registered users you have and what your growth rate is.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/GetBlimp/django-rest-framework-jwt/issues/463#issuecomment-470333789, or mute the thread https://github.com/notifications/unsubscribe-auth/AJN8OObi82v6teC4icjPABhg9Z60qQ4Oks5vUF5PgaJpZM4ZL3Iw .

Alex3917 commented 5 years ago

Oh I see what you're saying. I mean it makes sense to have that be a setting that points to a function, like all the other settings. No idea why that function would get removed as the default, it's possible the same function is just getting moved somewhere else or something so that's why you're getting warned not to monkey patch it.

fergyfresh commented 5 years ago

Ahhh. Its saying use the env var to set the output of that instead of monkey patching that function. got it now.