Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for auth against user models other than get_user_model() #96

Open
greyhare opened this issue Mar 21, 2019 · 10 comments
Open

Support for auth against user models other than get_user_model() #96

greyhare opened this issue Mar 21, 2019 · 10 comments

Comments

@greyhare
Copy link

greyhare commented Mar 21, 2019

I was looking through the code for this module, and I can't use it as-is because .state defines User = get_user_model(). It would be nice if I could authenticate different tables against different user models.

Rationale

In the system I'm building, I want the AUTH_USER_MODEL table to be "company employee" users only, and I want "customer" users to be in a separate table. (There may be company employees in the latter table, but we don't want their accounts connected.)

Also, django.contrib.auth makes demands on the user model that I don't want applied to the customer model, especially given the admin interface.

So, to do this, I'd like to just connect the appropriate authentication and permissions plugins for that table. Which means the things in .state probably need to not be globals.

Plan

I think I just need to take the global-dependent stuff and make a class out of it.

Existing functionality could be maintained by making the current functionality the default.

Do you have a better idea? Would you you interested in pull requests from this fork?

greyhare added a commit to greyhare/django-rest-framework-simplejwt that referenced this issue Mar 22, 2019
@greyhare
Copy link
Author

Well, that turned out to be much easier than anticipated. Passes all 122 tests. Now I need to figure out some tests for the custom user (as opposed to get_user_model) route. And add this to the README.

All I had to do was replace:

from .state import User

with:

from django.contrib.auth import get_user_model

# ...
class JWTAuthentication(...):
    # ...
    user_class = get_user_model()

Then the couple places that used User changed to use self.user_class instead. Nothing changes for existing uses of DRF_SJWT, and if you want to use a different user class, you just do:

from rest_framework_simplejwt.authentication import JWTAuthentication
from .models import Customer

class CustJWTAuthentication(JWTAuthentication):
    user_class = Customer

and then you just use CustJWTAuthentication instead of JWTAuthentication in your ViewSet'sauthentication_classes.

rest_framework_simplejwt.authentication.JWTTokenUserAuthentication required no changes.

The state.py file is no longer used and was removed. The token_backend stuff it had moved into tokens.Token similar to how User was moved above. The four tests that pulled in state were adjusted to remove the dependency (test_authentication.py was already changed).

@davesque
Copy link
Member

Actually, I really like where you're going with this. If you want to create a PR, I'll probably merge it in. I feel like having those class properties opens up the ability for people to more easily customize the way things work. It would be nice to have some tests that run through those customizations and then eventually some documentation to describe it.

@greyhare
Copy link
Author

In writing the tests, I'm noticing some other things like dependencies on api_settings, which can be resolved by similar means. So there's a bit more work to go.

But it's going well so far. Tricky mocking up a couple classes to act like a custom user table without actually making a Django model out of it (which would require populating tests/models.py and writing migrations and so forth.)

@greyhare
Copy link
Author

(i.e. the user ID field name needs to be overridable but default to api_settings.USER_ID_FIELD)

Python makes stuff too easy.

greyhare added a commit to greyhare/django-rest-framework-simplejwt that referenced this issue Mar 23, 2019
@greyhare
Copy link
Author

Well, only one tox test fails:

INTERNALERROR> ImportError: cannot import name 'six' from 'django.utils' ([...]/venv/src/djangorestframework-simplejwt/.tox/py37-djangomaster-drf39/lib/python3.7/site-packages/django/utils/__init__.py)

Seems like a nothingburger, really. Should I do anything about it?

@micurbanski
Copy link

How's that PR going @davesque? I did a very similar 'workaround' for my case and would appreciate having this in next release :)

@martyzz1
Copy link

martyzz1 commented May 4, 2019

yes +1 for this

@pawel-czarnecki
Copy link

Do we have some progress in this feature? Please for merge ;)

@iosifnicolae2
Copy link

Any news?

@Andrew-Chen-Wang
Copy link
Member

No progress as david doesn't use this anymore and I don't have time to add new features. PRs are welcome and appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants