Feat/signals #12
Feat/signals #12
Conversation
|
@imanhodjaev moving too fast this morning and opened this against the wrong repo, but I'll try to land it anyhow. |
|
@pnovotnak thank you for your contribution, I've been reviewing them 👍 |
django_auth0/models.py
Outdated
| meaning it should delete no data from the local database OR stormpath. | ||
| """ | ||
| if sync_groups: | ||
| sp_groups = [g.name for g in APPLICATION.groups] |
There was a problem hiding this comment.
Where this APPLICATION comes from?
There was a problem hiding this comment.
I see you took it from stormpath
APPLICATION = CLIENT.applications.get(settings.STORMPATH_APPLICATION)
It is going to brake things, may consider refactoring this.
django_auth0/models.py
Outdated
| return acc is not None | ||
| except StormpathError as e: | ||
| # explicity check to see if password is incorrect | ||
| if e.code == 7100: |
There was a problem hiding this comment.
I think these kind of magic literals (numbers, string should be defined as constants)
django_auth0/models.py
Outdated
| raise | ||
|
|
||
|
|
||
| class Auth0User(Auth0BaseUser): |
There was a problem hiding this comment.
If it is just an empty class then may be rename Auth0BaseUser to Auth0User?
| @@ -11,3 +12,23 @@ def get_config(): | |||
| 'AUTH0_CALLBACK_URL': settings.AUTH0_CALLBACK_URL, | |||
| 'AUTH0_SUCCESS_URL': settings.AUTH0_SUCCESS_URL, | |||
| } | |||
|
|
|||
|
|
|||
| def get_token(*, a0_config: dict=None, audience=None) -> str: | |||
There was a problem hiding this comment.
I think we need to have support for Python 2 and 3 at lease the next version should be the last one then we can just drop Python 2.x support. With typing annotations all users using Python 2.x will be broken.
django_auth0/models.py
Outdated
| @receiver(pre_save, sender=Group) | ||
| def save_group_to_auth0(sender, instance, **kwargs): | ||
| try: | ||
| if instance.pk is None: |
There was a problem hiding this comment.
Why not reverse it like
if instance.pk:
old_group = Group.objects.get(pk=instance.pk)
...
else:
APPLICATION.groups.create({'name': instance.name})
...There was a problem hiding this comment.
Again APPLICATION is stormpath specific thing.
| @@ -11,3 +12,23 @@ def get_config(): | |||
| 'AUTH0_CALLBACK_URL': settings.AUTH0_CALLBACK_URL, | |||
| 'AUTH0_SUCCESS_URL': settings.AUTH0_SUCCESS_URL, | |||
| } | |||
|
|
|||
|
|
|||
| def get_token(*, a0_config: dict=None, audience=None) -> str: | |||
There was a problem hiding this comment.
also dict is conflicting with builtin type I suggest to rename it.
|
Need to polish and may be test changes. |
|
Sorry, the state that the branch we have our branch at is good enough for the moment. I'm focused on other apps right now but I will be back on this soon. |
|
Sure, take you time. |
Provide signals that may be connected by the user to enable updating Auth0 when users are updated locally.