django-oauth-toolkit icon indicating copy to clipboard operation
django-oauth-toolkit copied to clipboard

Make Access Tokens Model Swappable

Open SpencerPinegar opened this issue 4 years ago • 6 comments

It is impossible to use a custom Model for OAUTH2_PROVIDER_ACCESS_TOKEN_MODEL (or any model other than Application) even though in the documentation there is a setting to customize the model and a custom model access method; this is extremely misleading and has led to 2 different feature requests for DOT about this issue in the last year (#487 #634), along with some stack-overflow issues; example.

This problem is caused because of circular dependencies on foreign keys not being handled correctly in the migration. This makes defining and using custom models for Access/Refresh Tokens impossible out of the box - but the documentation makes it seem like an included feature.

I suggest the project uses the same API Django uses to make the User model swappable - django-swappable-models. This would remove the circular dependencies and should make customizing any Model (Application, Refresh Token, Access Token, Grant, etc) work regardless of how migrations are run out of the box just like Django's User Model - the only thing the programmer would have to do is specify the model in settings. To do this:

  1. update Meta attribute swappable for all Abstract Models to use swapper.swappable_setting API method
  2. update ForeignKey dependencies for all Abstract Models to use swapper.get_model_name API method
  3. update get_***_model method to use swapper.load_model API method
  4. update migration scripts swappable_dependency model to user swapper.dependency API method
  5. update migration scripts swappable option to use swapper.swappable_setting API method
  6. add swapper to setup.cfg
  7. add swapper to requirements.txt

The current implementation only works for AbstractApplication Model, is dependent on the ordering of the migrations, and requires customization of the migration file after it is run to ensure it works; All of these are undesirable. The work arounds in #487, #634 require custom migration rewrites, are still order dependent, and have more boilerplate code; this is also undesirable.

This feature will make the documentation reflect the project more accurately and simplify using and customizing this app in the future. I am going to make a PR for this issue, let me know about what you think!

SpencerPinegar avatar Sep 17 '20 18:09 SpencerPinegar

Here is my PR - https://github.com/jazzband/django-oauth-toolkit/pull/872 - looking for some help to get this over the line and merged in so this migration issue can be resolved permanently.

SpencerPinegar avatar Sep 17 '20 22:09 SpencerPinegar

@SpencerPinegar Can you take a look at this documentation and see if what I did makes sense?

n2ygk avatar Oct 14 '20 13:10 n2ygk

@n2ygk The link you provided throws a 404. I believe that this is the documentation link you meant to share:

https://columbia-it-django-jsonapi-training.readthedocs.io/en/latest/oidc/#advanced-topic-extending-the-dot-accesstoken-model

Looks like the problem in the link was ~/oidc.html~ -> /odic/

I was able to follow this documentation and get the swapped models working, so it made sense to me. One minor improvement could be to include the entire migration file instead of omitting the migrations for MyRefreshToken and MyApplication. That part was a little confusing. Thanks!

petermorrowdev avatar Feb 06 '21 21:02 petermorrowdev

Hi @petermorrow This is what I get after all the successful migrations:

AttributeError: Manager isn't available; 'oauth2_provider.Application' has been swapped for 'oauth.Application'

denniel-sadian avatar Feb 11 '21 12:02 denniel-sadian

Any updates?

krzysieqq avatar Jul 09 '22 18:07 krzysieqq

Any updates?

+1

R70YNS avatar Sep 05 '22 13:09 R70YNS