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

Allow TokenUser class to be configurable #172

Merged
merged 1 commit into from
Feb 28, 2020

Conversation

ajhodges
Copy link
Contributor

@ajhodges ajhodges commented Nov 6, 2019

Addresses #171 , adding a setting TOKEN_USER_CLASS, that can be used to override the default TokenUser. Added a unit test to ensure that this works, and there's an existing assertion that the default TokenUser class is preserved in TestJWTTokenUserAuthentication.test_get_user.

@ajhodges
Copy link
Contributor Author

ajhodges commented Nov 6, 2019

Also pretty please requesting a new release if/when this gets merged :)

Copy link

@KimSoungRyoul KimSoungRyoul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be required feature

This way we don't have to override

@ajhodges
Copy link
Contributor Author

@davesque any feedback on this PR?

@jrsupplee
Copy link

Why not just create your own authentication class with a get_user method that returns the desired token?

@ajhodges
Copy link
Contributor Author

We certainly could make our own authentication class that just uses a different user, but, as shown by this PR, it's also fairly trivial to add support to make this easily configurable via settings. I prefer to not re-write code if I don't need to. It's always a technical-debt-win in my book if I can rely on an externally maintained package vs forking/reimplementing it on my own.

I feel like a setting is appropriate for this, and judging by the ':+1:'s I'm not alone. Is there an issue you have with this solution?

@jvhellemondt
Copy link

This would be great. I have the issue that I cannot exclude
'django.contrib.auth', 'django.contrib.contenttypes',

in my settings due to this.

Copy link
Member

@davesque davesque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@davesque davesque merged commit 5c6a60b into jazzband:master Feb 28, 2020
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

Successfully merging this pull request may close these issues.

5 participants