addressed PR comments

Author: daniel-sanche

google/auth/crypt/rsa.py

@@ -33,14 +33,8 @@
 class RSAVerifier(base.Verifier):
     """Verifies RSA cryptographic signatures using public keys.
 
-    Requires installation of `cryptography` optional dependency.
-
-    .. deprecated::
-        The `rsa` library has been archived. Please migrate to
-        `cryptography` for public keys.
-
     Args:
-        public_key (Union[rsa.key.PublicKey, cryptography.hazmat.primitives.asymmetric.rsa.RSAPublicKey]):
+        public_key (Union["rsa.key.PublicKey", cryptography.hazmat.primitives.asymmetric.rsa.RSAPublicKey]):
             The public key used to verify signatures.
     Raises:
         ValueError: if an unrecognized public key is provided
@@ -83,14 +77,8 @@ def from_string(cls, public_key):
 class RSASigner(base.Signer, base.FromServiceAccountMixin):
     """Signs messages with an RSA private key.
 
-    Requires installation of `cryptography` optional dependency.
-
-    .. deprecated::
-        The `rsa` library has been archived. Please migrate to
-        `cryptography` for public keys.
-
     Args:
-        private_key (Union[rsa.key.PrivateKey, cryptography.hazmat.primitives.asymmetric.rsa.RSAPrivateKey]):
+        private_key (Union["rsa.key.PrivateKey", cryptography.hazmat.primitives.asymmetric.rsa.RSAPrivateKey]):
             The private key to sign with.
         key_id (str): Optional key ID used to identify this private key. This
             can be useful to associate the private key with its associated

setup.py

@@ -18,19 +18,18 @@
 from setuptools import find_namespace_packages
 from setuptools import setup
 
+cryptography_base_require = [
+    "cryptography >= 38.0.3",
+]
 
 DEPENDENCIES = (
     "pyasn1-modules>=0.2.1",
-    "cryptography >= 38.0.3",
-    # TODO: remove rsa from dependencies in next release (replaced with cryptography)
+    cryptography_base_require,
+    # TODO: remove rsa from dependencies in next release (replaced with cryptography)i
+    # https://github.com/googleapis/google-auth-library-python/issues/1810
     "rsa>=3.1.4,<5",
 )
 
-# Note: cryptography was made into a required dependency. Extra is kept for backwards compatibility
-cryptography_extra_require = [
-    "cryptography >= 38.0.3",
-]
-
 requests_extra_require = ["requests >= 2.20.0, < 3.0.0"]
 
 aiohttp_extra_require = ["aiohttp >= 3.6.2, < 4.0.0", *requests_extra_require]
@@ -77,7 +76,8 @@
 ]
 
 extras = {
-    "cryptography": cryptography_extra_require,
+    # Note: cryptography was made into a required dependency. Extra is kept for backwards compatibility
+    "cryptography": cryptography_base_require,
     "aiohttp": aiohttp_extra_require,
     "enterprise_cert": enterprise_cert_extra_require,
     "pyopenssl": pyopenssl_extra_require,

tests/crypt/test_rsa.py

@@ -27,44 +27,60 @@
 
 DATA_DIR = os.path.join(os.path.dirname(__file__), "..", "data")
 
-with open(os.path.join(DATA_DIR, "privatekey.pem"), "rb") as fh:
-    PRIVATE_KEY_BYTES = fh.read()
-    CRYPTOGRAPHY_PRIVATE_KEY = serialization.load_pem_private_key(
-        PRIVATE_KEY_BYTES, password=None, backend=backends.default_backend()
+@pytest.fixture
+def private_key_bytes():
+    with open(os.path.join(DATA_DIR, "privatekey.pem"), "rb") as fh:
+        return fh.read()
+
+@pytest.fixture
+def public_key_bytes():
+    with open(os.path.join(DATA_DIR, "privatekey.pub"), "rb") as fh:
+        return fh.read()
+
+@pytest.fixture
+def cryptography_private_key(private_key_bytes):
+    return serialization.load_pem_private_key(
+        private_key_bytes, password=None, backend=backends.default_backend()
     )
-    RSA_PRIVATE_KEY = rsa_lib.PrivateKey.load_pkcs1(PRIVATE_KEY_BYTES)
 
-with open(os.path.join(DATA_DIR, "privatekey.pub"), "rb") as fh:
-    PUBLIC_KEY_BYTES = fh.read()
-    CRYPTOGRAPHY_PUBLIC_KEY = serialization.load_pem_public_key(
-        PUBLIC_KEY_BYTES, backend=backends.default_backend()
+@pytest.fixture
+def rsa_private_key(private_key_bytes):
+    return rsa_lib.PrivateKey.load_pkcs1(private_key_bytes)
+
+@pytest.fixture
+def cryptography_public_key(public_key_bytes):
+    return serialization.load_pem_public_key(
+        public_key_bytes, backend=backends.default_backend()
     )
-    RSA_PUBLIC_KEY = rsa_lib.PublicKey.load_pkcs1(PUBLIC_KEY_BYTES)
+
+@pytest.fixture
+def rsa_public_key(public_key_bytes):
+    return rsa_lib.PublicKey.load_pkcs1(public_key_bytes)
 
 
 class TestRSAVerifier:
-    def test_init_with_cryptography_key(self):
-        verifier = rsa.RSAVerifier(CRYPTOGRAPHY_PUBLIC_KEY)
+    def test_init_with_cryptography_key(self, cryptography_public_key):
+        verifier = rsa.RSAVerifier(cryptography_public_key)
         assert isinstance(verifier._impl, _cryptography_rsa.RSAVerifier)
-        assert verifier._impl._pubkey == CRYPTOGRAPHY_PUBLIC_KEY
+        assert verifier._impl._pubkey == cryptography_public_key
 
-    def test_init_with_rsa_key(self):
-        verifier = rsa.RSAVerifier(RSA_PUBLIC_KEY)
+    def test_init_with_rsa_key(self, rsa_public_key):
+        verifier = rsa.RSAVerifier(rsa_public_key)
         assert isinstance(verifier._impl, _python_rsa.RSAVerifier)
-        assert verifier._impl._pubkey == RSA_PUBLIC_KEY
+        assert verifier._impl._pubkey == rsa_public_key
 
-    def test_warning_with_rsa(self):
+    def test_warning_with_rsa(self, rsa_public_key):
         with pytest.warns(DeprecationWarning, match="The 'rsa' library is deprecated"):
-            rsa.RSAVerifier(RSA_PUBLIC_KEY)
+            rsa.RSAVerifier(rsa_public_key)
 
     def test_init_with_unknown_key(self):
         unknown_key = object()
 
         with pytest.raises(ValueError):
             rsa.RSAVerifier(unknown_key)
 
-    def test_verify_delegates(self):
-        verifier = rsa.RSAVerifier(CRYPTOGRAPHY_PUBLIC_KEY)
+    def test_verify_delegates(self, cryptography_public_key):
+        verifier = rsa.RSAVerifier(cryptography_public_key)
 
         # Mock the implementation's verify method
         with mock.patch.object(
@@ -75,41 +91,41 @@ def test_verify_delegates(self):
             mock_verify.assert_called_once_with(b"message", b"signature")
 
     @mock.patch("google.auth.crypt.rsa._cryptography_rsa")
-    def test_from_string_cryptography(self, mock_crypto):
+    def test_from_string_cryptography(self, mock_crypto, public_key_bytes):
         expected_verifier = mock.Mock()
         mock_crypto.RSAVerifier.from_string.return_value = expected_verifier
 
-        result = rsa.RSAVerifier.from_string(PUBLIC_KEY_BYTES)
+        result = rsa.RSAVerifier.from_string(public_key_bytes)
 
         assert result._impl == expected_verifier
-        mock_crypto.RSAVerifier.from_string.assert_called_once_with(PUBLIC_KEY_BYTES)
+        mock_crypto.RSAVerifier.from_string.assert_called_once_with(public_key_bytes)
 
 
 class TestRSASigner:
-    def test_init_with_cryptography_key(self):
-        signer = rsa.RSASigner(CRYPTOGRAPHY_PRIVATE_KEY, key_id="123")
+    def test_init_with_cryptography_key(self, cryptography_private_key):
+        signer = rsa.RSASigner(cryptography_private_key, key_id="123")
         assert isinstance(signer._impl, _cryptography_rsa.RSASigner)
-        assert signer._impl._key == CRYPTOGRAPHY_PRIVATE_KEY
+        assert signer._impl._key == cryptography_private_key
         assert signer._impl.key_id == "123"
 
-    def test_init_with_rsa_key(self):
-        signer = rsa.RSASigner(RSA_PRIVATE_KEY, key_id="123")
+    def test_init_with_rsa_key(self, rsa_private_key):
+        signer = rsa.RSASigner(rsa_private_key, key_id="123")
         assert isinstance(signer._impl, _python_rsa.RSASigner)
-        assert signer._impl._key == RSA_PRIVATE_KEY
+        assert signer._impl._key == rsa_private_key
         assert signer._impl.key_id == "123"
 
-    def test_warning_with_rsa(self):
+    def test_warning_with_rsa(self, rsa_private_key):
         with pytest.warns(DeprecationWarning, match="The 'rsa' library is deprecated"):
-            rsa.RSASigner(RSA_PRIVATE_KEY, key_id="123")
+            rsa.RSASigner(rsa_private_key, key_id="123")
 
     def test_init_with_unknown_key(self):
         unknown_key = object()
 
         with pytest.raises(ValueError):
             rsa.RSASigner(unknown_key)
 
-    def test_sign_delegates(self):
-        signer = rsa.RSASigner(RSA_PRIVATE_KEY)
+    def test_sign_delegates(self, rsa_private_key):
+        signer = rsa.RSASigner(rsa_private_key)
 
         with mock.patch.object(
             signer._impl, "sign", return_value=b"signature"
@@ -119,32 +135,32 @@ def test_sign_delegates(self):
             mock_sign.assert_called_once_with(b"message")
 
     @mock.patch("google.auth.crypt.rsa._cryptography_rsa")
-    def test_from_string_delegates_to_cryptography(self, mock_crypto):
+    def test_from_string_delegates_to_cryptography(self, mock_crypto, private_key_bytes):
         expected_signer = mock.Mock()
         mock_crypto.RSASigner.from_string.return_value = expected_signer
 
-        result = rsa.RSASigner.from_string(PRIVATE_KEY_BYTES, key_id="123")
+        result = rsa.RSASigner.from_string(private_key_bytes, key_id="123")
 
         assert result._impl == expected_signer
         mock_crypto.RSASigner.from_string.assert_called_once_with(
-            PRIVATE_KEY_BYTES, key_id="123"
+            private_key_bytes, key_id="123"
         )
 
-    def test_end_to_end_cryptography_lib(self):
-        signer = rsa.RSASigner.from_string(PRIVATE_KEY_BYTES)
+    def test_end_to_end_cryptography_lib(self, private_key_bytes, public_key_bytes):
+        signer = rsa.RSASigner.from_string(private_key_bytes)
         message = b"Hello World"
         sig = signer.sign(message)
-        verifier = rsa.RSAVerifier.from_string(PUBLIC_KEY_BYTES)
+        verifier = rsa.RSAVerifier.from_string(public_key_bytes)
         result = verifier.verify(message, sig)
         assert result is True
         assert isinstance(verifier._impl, _cryptography_rsa.RSAVerifier)
         assert isinstance(signer._impl, _cryptography_rsa.RSASigner)
 
-    def test_end_to_end_rsa_lib(self):
-        signer = rsa.RSASigner(RSA_PRIVATE_KEY)
+    def test_end_to_end_rsa_lib(self, rsa_private_key, rsa_public_key):
+        signer = rsa.RSASigner(rsa_private_key)
         message = b"Hello World"
         sig = signer.sign(message)
-        verifier = rsa.RSAVerifier(RSA_PUBLIC_KEY)
+        verifier = rsa.RSAVerifier(rsa_public_key)
         result = verifier.verify(message, sig)
         assert bool(result) is True
         assert isinstance(verifier._impl, _python_rsa.RSAVerifier)