Make Resource Owner model configurable #459
Conversation
- change relevant ForeignKeys - add resource owner extractors in backend and validator - add configurable user lookup in views
|
I'm torn. On the one hand this a pretty common pattern, but on the other hand we've talked about making all the token models etc abstract so that you can swap them out for your own version. I'm +0 on this. We could have both. |
jleclanche
force-pushed
the
master
branch
2 times, most recently
from
14cfaa3 to
92311a5
Compare
|
@jleclanche I understand, I had a closer look at #252 and #347 and it would indeed make it easier to swap in this pattern. What would be needed to make either solution go forward ? |
| :param credentials: Authorization credentials dictionary containing | ||
| `client_id`, `state`, `redirect_uri`, `response_type` | ||
| :param allow: True if the user authorize the client, otherwise False | ||
| :param allow: True if the resource owner authorize the client, otherwise False |
| UserModel = get_user_model() | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Nit: This extra newline is not necessary.
|
|
||
| from ..models import get_application_model, Grant, AccessToken, RefreshToken | ||
| from ..models import get_application_model, get_resource_owner_model | ||
| from ..models import Grant, AccessToken, RefreshToken |
There was a problem hiding this comment.
These two lines can be combined.
| self.user = UserModel.objects.create_user("test_user", "test@user.com", "123456") | ||
|
|
||
| def test_model(self): | ||
| self.user.resource_owners.all() |
There was a problem hiding this comment.
Add a comment explaining what you're testing here.
jleclanche
force-pushed
the
master
branch
2 times, most recently
from
80a3973 to
f59509f
Compare
|
@jleclanche maybe you will be interested on reviewing this one? #467 |
|
As the swappable models feature is already pushed to master, could we work on this? I can help you if needed. Maybe adding a detailed entry in the docs on how to use swappable models to accomplish this behavior can be enough? |
|
I'll always take docs PRs. I still think they're not mutually exclusive, so if you guys still consider this wanted, it'll have to be rebased. |
|
This kind of stalled on my end. The migration I pushed doesn't work when
the FK changes type with a db that has proper constraints (it fails
pointing to an uuid pk'ed model in postgres for example, the type cannot be
upcasted) , and the migration process isn't really complete.
Le 28 avr. 2017 22:11, "Jerome Leclanche" <notifications@github.com> a
écrit :
… I'll always take docs PRs. I still think they're not mutually exclusive,
so if you guys still consider this wanted, it'll have to be rebased.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#459 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABnLzroX5RUJcLBHVFlPHLzyTfpjh85Xks5r0kfXgaJpZM4MXhS7>
.
|
|
Closing since this has stalled for too long and isn't ready to land. |
5 participants
Hello there,
Following my (unanswered) thread on google groups regarding the possibility to have another model than Django's AUTH_USER_MODEL as the Resource Owner for tokens (I've used django-organizations OrganizationUser as an example), I've been experimenting a little bit with the idea.
This PR attempts to implement a configurable setting allowing developers to change the model considered as Resource Owner for Grants, AccessToken and RefreshToken.
Usage:
OAUTH2_PROVIDER_RESOURCE_OWNER_MODEL = 'myapp.MyResourceOwner'_extract_resource_ownermethod, to get a resource owner instance from the requestIt's been a bit hard for me to follow the path of a request in the multiple reciprocal calls between DOT and OAuthLib, so I don't really know how far I am from a complete implementation. This patch works for my use case and passes existing tests.
What do you think ? Is it a sane way to achieve this kind of flexibility ? Is it desirable in DOT ?
Please review and comment.