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

feature: adding support custom username field #368

Merged
merged 3 commits into from
Apr 4, 2024

Conversation

InzGIBA
Copy link
Contributor

@InzGIBA InzGIBA commented Mar 26, 2024

I faced a problem when implementing this package in my project. The administrator panel worked perfectly when authorizing, but for example package simplejwt (drf) sent data based on a custom field of the user.

Link: https://github.com/jazzband/djangorestframework-simplejwt/blob/c791e987332ed5e22a86428160d6372b1d85ffae/rest_framework_simplejwt/serializers.py#L41

In this pull-request we added a check for the "username" field

Link to Django ModelBackend: https://github.com/django/django/blob/ae10146793dca0e0594c7acdee20ca3810983f39/django/contrib/auth/backends.py#L38

@francoisfreitag
Copy link
Collaborator

The suggestion looks fine to me. Could you please add a test to cover the added code path?

@francoisfreitag
Copy link
Collaborator

I see that tests now fail, because the code proceeds with username=None in case the fallback yields nothing else.

@InzGIBA InzGIBA marked this pull request as draft March 28, 2024 06:16
@InzGIBA
Copy link
Contributor Author

InzGIBA commented Mar 28, 2024

  • Fixed a bug that prevented all previously written tests from being run in projects
  • Added a test "test_auth_custom_field" to test a different argument in the "authenticate" function, in turn the test was based on an example of another test "test_get_custom_field"

@InzGIBA InzGIBA marked this pull request as ready for review March 28, 2024 07:05
Copy link
Collaborator

@francoisfreitag francoisfreitag left a comment

Choose a reason for hiding this comment

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

Looking great, thanks!

@francoisfreitag francoisfreitag merged commit aa04ca5 into django-auth-ldap:master Apr 4, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants