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

feat(auth): add stub for BasePasswordHasher decode method and salt_entropy variable #1647

Conversation

Kadermiyanyedi
Copy link
Contributor

Hi! I was looking at the Todo list and thought I could help with this.

  • I made additions for the salt_entropy variable and the decode method in the BasePasswordHasher class.
    *Also removed the algorithm and variable variables that were added before but included in the todo list.

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.

Hi, thanks a lot for your help!

But, there are some problems yet to fix:

error: django.contrib.auth.hashers.BasePasswordHasher.algorithm variable differs from runtime type None
Stub: in file /home/runner/work/django-stubs/django-stubs/django-stubs/contrib/auth/hashers.pyi:18
builtins.str
Runtime:
None

error: django.contrib.auth.hashers.BasePasswordHasher.library variable differs from runtime type None
Stub: in file /home/runner/work/django-stubs/django-stubs/django-stubs/contrib/auth/hashers.pyi:19
Union[builtins.str, tuple[builtins.str, builtins.str]]
Runtime:
None

note: unused allowlist entry django.contrib.auth.hashers.Argon2PasswordHasher.decode
note: unused allowlist entry django.contrib.auth.hashers.BCryptSHA256PasswordHasher.decode
note: unused allowlist entry django.contrib.auth.hashers.CryptPasswordHasher.decode
note: unused allowlist entry django.contrib.auth.hashers.MD5PasswordHasher.decode
note: unused allowlist entry django.contrib.auth.hashers.PBKDF2PasswordHasher.decode
note: unused allowlist entry django.contrib.auth.hashers.SHA1PasswordHasher.decode
note: unused allowlist entry django.contrib.auth.hashers.UnsaltedMD5PasswordHasher.decode
note: unused allowlist entry django.contrib.auth.hashers.UnsaltedSHA1PasswordHasher.decode

@Kadermiyanyedi Kadermiyanyedi force-pushed the new-stub-basepasswordhasher-decode-method branch 3 times, most recently from 3464db8 to 34b6295 Compare August 11, 2023 15:17
@Kadermiyanyedi
Copy link
Contributor Author

Hi, thanks for your response. I checked the Django source code again.

-> I added the decode function to the subclasses if the function return type is different from the BasePasswordHasher class

-> The algorithm and library variables are set to None in the BasePasswordHasher class in Django source code. The library variable is set to string or tuple[string] only in BCryptSHA256PasswordHasher and Argon2PasswordHasher classes. I set to None the library variable in the BasePasswordHasher class to None and I added library variable to two sub classes.

And finally I removed these changes from the todo list file.

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.

Maybe like this?

django-stubs/contrib/auth/hashers.pyi Outdated Show resolved Hide resolved
django-stubs/contrib/auth/hashers.pyi Outdated Show resolved Hide resolved
@Kadermiyanyedi Kadermiyanyedi force-pushed the new-stub-basepasswordhasher-decode-method branch from 34b6295 to 7835df7 Compare August 12, 2023 09:02
@Kadermiyanyedi
Copy link
Contributor Author

Hi, @sobolevn do you have any other suggestions? I can do :)

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.

No, LGTM! Thank you

@sobolevn sobolevn merged commit 712fa82 into typeddjango:master Aug 14, 2023
28 checks passed
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.

2 participants