Skip to content

Adjusted token admin to map to user ID. #7341

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 15, 2020
Merged

Adjusted token admin to map to user ID. #7341

merged 1 commit into from
Jun 15, 2020

Conversation

carltongibson
Copy link
Collaborator

Closes #6131.

  • Adds a proxy model for Token that uses the user.pk, rather than it's own.
  • Adjusts Admin to map back from User ID to token instance.

Proof of concept. Works but...

  • Needs some unit tests.
  • Could do with testing against a custom user model.

Thoughts?

@carltongibson carltongibson requested a review from tomchristie May 17, 2020 14:34
@carltongibson carltongibson mentioned this pull request May 17, 2020
Closed
6 tasks
@carltongibson
Adjusted token admin to map to user ID.
27e9279 
Closes encode#6131.

* Adds a proxy model for Token that uses the user.pk, rather than it's own.
* Adjusts Admin to map back from User ID to token instance.
@tomchristie tomchristie added this to the 3.12 Release milestone May 28, 2020
@tomchristie
Copy link
Member

Righty, tested this all locally, both with a standard user model, and with a custom user model.
Looks great, let's do it!

@tomchristie tomchristie merged commit e2bd3b6 into encode:master Jun 15, 2020
@kalekseev
Copy link
Contributor

Django complains about TokenProxy "TypeError: Abstract base class containing model fields not permitted for proxy model 'TokenProxy'.", Token is abstract model if the app is not installed

@carltongibson
Copy link
Collaborator Author

@kalekseev "If the app is not installed" -- what does that mean exactly? (Is "install it" an appropriate fix?)

Probably need a new issue with an example project.

@kalekseev
Copy link
Contributor

@carltongibson sorry for the lack of details I was from mobile. Basically Token become abstract if 'rest_framework.authtoken' not in settings.INSTALLED_APPS https://github.com/encode/django-rest-framework/blob/master/rest_framework/authtoken/models.py#L26. In my case rest_auth app importing rest_framework.authtoken.models but it's not used in my project so it's not in installed apps. Same thing will happen if someone needs a CustomToken that extends Token in that case he wants Token model to be abstract.

@carltongibson
Copy link
Collaborator Author

So we could maybe fix this making proxy depend on the same conditional...?

Would you be up for adding a PR for that?

@kalekseev
Copy link
Contributor

@carlfarrington I have discovered that problem developing this pr #7438 and I don't have that much free time right now to commit myself to one more.

@carltongibson
Copy link
Collaborator Author

Ok, in that case, can I ask you to open a new issue, so this doesn't get lost in the cracks? Thanks!

@kalekseev kalekseev mentioned this pull request Jul 30, 2020
Closed
6 tasks
@kevin-brown kevin-brown mentioned this pull request Dec 7, 2020
Closed
2 tasks
sigvef pushed a commit to sigvef/django-rest-framework that referenced this pull request Dec 3, 2022
@carltongibson @sigvef
Adjusted token admin to map to user ID. (encode#7341)
aea86f0 
Closes encode#6131.

* Adds a proxy model for Token that uses the user.pk, rather than it's own.
* Adjusts Admin to map back from User ID to token instance.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomchristie tomchristie Awaiting requested review from tomchristie

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.12 Release
Development

Successfully merging this pull request may close these issues.

Token admin page leaks access tokens into log files
3 participants
@carltongibson @tomchristie @kalekseev