fixed DeprecationWarning in requests use in auth.py #2570
Conversation
| @@ -4,10 +4,10 @@ | |||
| mslib.utils.auth | |||
| ~~~~~~~~~~~~~~~~ | |||
|
|
|||
| handles passwords from the keyring for login and http_auuth | |||
| handles passwords from the keyring for login and http_auth | |||
There was a problem hiding this comment.
good, our spellchecker had not found that :)
| @@ -100,14 +98,21 @@ def get_auth_from_url_and_name(server_url, http_auth, overwrite_login_cache=True | |||
| if server_url == url: | |||
| try: | |||
| password = get_password_from_keyring(service_name=url, username=auth_name) | |||
| if password is None: | |||
There was a problem hiding this comment.
We don't allow to save empty strings as password, that is not a good idea, to remove None here.
| auth = constants.AUTH_LOGIN_CACHE.get(server_url, (name, None)) | ||
|
|
||
| # Always return a valid string for the password | ||
| auth = constants.AUTH_LOGIN_CACHE.get(server_url, (name, "")) |
There was a problem hiding this comment.
We don't accept empty passwords. And should not start using them in the AUTH_LOGIN_CACHE.
|
When we have an example how we submit None over requests, then we should catch that where we did that. Because there will be no login possible with an empty or None string. It is not needed to submit such values at all. |
|
Respected ReimarBauer , I saw your message late at night and wanted to keep you updated. As I’m still a beginner with open source, I’ll need a bit more time to review the code and understand my mistakes. I’ll be working on it tomorrow and will reach out as soon as possible with any new solutions or updates. |
|
Welcome @YogeshRathee512 there are two options currently where we use the current functionality.
see if it is necessary to intercept the sending of None here. You can look at it via debugging, or via a test. |
Purpose of PR?:
Fixes #1787
Resolves the DeprecationWarning for non-string passwords in requests when using keyring.
Does this PR introduce a breaking change?
No, it ensures future compatibility with requests 3.0.0 by handling password strings correctly.
If the changes in this PR are manually verified, list down the scenarios covered::
Verified that credentials are correctly retrieved from keyring.
Checked that the system no longer triggers the DeprecationWarning.
Tested login flow for HTTP authentication using the stored credentials.
Ensured that errors are handled properly when keyring is unavailable or credentials do not exist.
Additional information for reviewer?:
This PR is part of a follow-up on improving error handling and avoiding deprecated practices with third-party libraries such as requests. It is a continuation of previous work on authentication refactoring.
Does this PR result in some Documentation changes?
No.
Code Changes:
Initial Code:
Updated Code:
Checklist:
Bug fix. Fixes #1787
New feature (Non-API breaking changes that add functionality)
PR Title follows the convention of :
Commit has unit tests