fix: replace rsa dependency with cryptography#1771
fix: replace rsa dependency with cryptography#1771sai-sunder-s wants to merge 29 commits intomainfrom
Conversation
This commit removes the `rsa` library as a dependency and makes the `cryptography` library a required, core dependency. Previously, `cryptography` was an optional dependency, and the library would fall back to a pure Python RSA implementation using the `rsa` library if `cryptography` was not installed. Changes made: - Modified `setup.py` to remove `rsa` from dependencies and add `cryptography` with version constraints. - Updated `google/auth/crypt/rsa.py` to directly use the `cryptography`-based RSA implementation (`_cryptography_rsa.py`) and remove the fallback mechanism. - Removed the pure Python RSA implementation file (`google/auth/crypt/_python_rsa.py`). - Removed the corresponding tests for the pure Python RSA implementation (`tests/crypt/test__python_rsa.py`). Core unit tests pass after these changes.
parthea
left a comment
There was a problem hiding this comment.
LGTM. We should mark this as fix instead of refactor so that the change to setup.py is visible in release notes.
fix: add dependency on cryptography
fix: drop dependency on rsa
docs/conf.py
Outdated
|
|
||
| def autodoc_skip_member_handler(app, what, name, obj, skip, options): | ||
| """ | ||
| Skips members from internal modules (like _cryptography_rsa or base) |
There was a problem hiding this comment.
Please could you file a bug for issue that requires this docs workaround, even if it will be closed with the changes in this PR. Add a link to the issue in this comment.
docs/conf.py
Outdated
| if public_obj is obj: | ||
| return True # Skip this internal one | ||
| except ImportError: | ||
| pass # Should not happen if the library is installed |
There was a problem hiding this comment.
If this is not expected to happen, can we just let the error bubble up?
docs/conf.py
Outdated
| pass # Should not happen if the library is installed | ||
|
|
||
| # Handle Signer and Verifier from base | ||
| elif name in ("Signer", "Verifier") and hasattr(obj, "__module__"): |
There was a problem hiding this comment.
This code under this block is similar to the above if statement. Is it possible to refactor it?
|
Any update here? rsa (https://stuvel.eu/software/rsa/) is officially archived. |
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
|
I looked a bit closer at the code, and it looks like this change would require a breaking change. Here's an example of some code that would break after this update: I don't think there's a way to keep this flow consistent, without requiring rsa as a dependency One thing we could do is make both rsa and cryptography optional dependencies, and raise an ImportError if both are missing? This would still be a breaking change, since they'll need to update their requirements.txt files. But they could keep using the rsa code in the mean-time. I think it might be better to do a clean break from rsa though, since it's an archived package. I guess we could instruct users how to copy the old RSASigner/RSAVerifier classes into their codebase if they still want them |
Please note that this project was archived by the maintainer: sybrenstuvel/python-rsa@42b0e14 The only buildroot package using python-rsa is python-google-auth which did not switch to an alternative yet: googleapis/google-auth-library-python#1771 Signed-off-by: Bernd Kuhls <bernd@kuhls.net> Signed-off-by: Julien Olivain <ju.o@free.fr>
|
Switching to draft as there are merge conflicts |
|
Closing in favor of #1929 |
This commit removes the
rsalibrary as a dependency and makes thecryptographylibrary a required, core dependency.Previously,
cryptographywas an optional dependency, and the library would fall back to a pure Python RSA implementation using thersalibrary ifcryptographywas not installed.Changes made:
setup.pyto removersafrom dependencies and addcryptographywith version constraints.google/auth/crypt/rsa.pyto directly use thecryptography-based RSA implementation (_cryptography_rsa.py) and remove the fallback mechanism.google/auth/crypt/_python_rsa.py).tests/crypt/test__python_rsa.py).Core unit tests pass after these changes.