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

Fix signature of RemoteUserBackend.configure_user #532

Merged

Conversation

lambdalisue
Copy link
Contributor

I have made things!

The function signature of django.contrib.auth.backends.RemoteUserBackend.configure_user has changed from 2.1 on 2.2 but django-stubs did not follow that.

stable/2.1.x

https://github.com/django/django/blob/stable/2.1.x/django/contrib/auth/backends.py#L163

    def configure_user(self, user):
        """
        Configure a user after creation and return the updated user.
        By default, return the user unmodified.
        """
        return user
stable/2.2.x ~ stable/3.1.x

https://github.com/django/django/blob/stable/2.2.x/django/contrib/auth/backends.py#L177
https://github.com/django/django/blob/stable/3.0.x/django/contrib/auth/backends.py#L236
https://github.com/django/django/blob/stable/3.1.x/django/contrib/auth/backends.py#L222

    def configure_user(self, request, user):
        """
        Configure a user after creation and return the updated user.
        By default, return the user unmodified.
        """
        return user

Related issues

I couldn't found any.

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@@ -35,6 +35,6 @@ class AllowAllUsersModelBackend(ModelBackend): ...
class RemoteUserBackend(ModelBackend):
create_unknown_user: bool = ...
def clean_username(self, username: str) -> str: ...
def configure_user(self, user: User) -> User: ...
def configure_user(self, request: Any, user: User) -> User: ...
Copy link
Member

Choose a reason for hiding this comment

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

I guess it should be request: HttpRequest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The function is used in authenticate() method and the request type, which is directly passed, in that function is Any

https://github.com/django/django/blob/stable/2.2.x/django/contrib/auth/backends.py#L150-L160
https://github.com/typeddjango/django-stubs/blob/master/django-stubs/contrib/auth/backends.pyi#L14

Shall I

  1. Let it go?
  2. Fix type definition of authenticate() as well?

I'm not sure if request in authenticate() is HttpRequest or can be others.

Copy link
Member

Choose a reason for hiding this comment

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

Any ideas what else can it be?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No idea. I just said I'm not sure while I didn't wrote the definition of authenticate().

Copy link
Member

Choose a reason for hiding this comment

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

let's change both then!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sobolevn Done

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@sobolevn sobolevn merged commit f3e0872 into typeddjango:master Nov 19, 2020
@lambdalisue lambdalisue deleted the support-remote-user-on-django-22 branch November 19, 2020 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants