Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 27 additions & 18 deletions packages/google-auth/google/auth/crypt/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,35 @@
"""

from google.auth.crypt import base
from google.auth.crypt import es
from google.auth.crypt import es256
from google.auth.crypt import rsa

EsSigner = es.EsSigner
EsVerifier = es.EsVerifier
ES256Signer = es256.ES256Signer
ES256Verifier = es256.ES256Verifier
# google.auth.crypt.es depends on the crytpography module which may not be
# successfully imported depending on the system.
try:
from google.auth.crypt import es
from google.auth.crypt import es256
except ImportError: # pragma: NO COVER
es = None # type: ignore
es256 = None # type: ignore
Comment on lines +49 to +50
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Following the repository's general rules, use the more specific # type: ignore[assignment] when assigning None to optional dependencies in an except ImportError block to improve type checking precision.

Suggested change
es = None # type: ignore
es256 = None # type: ignore
es = None # type: ignore[assignment]
es256 = None # type: ignore[assignment]
References
  1. When handling an optional dependency by assigning None in an except ImportError block, type: ignore[assignment] is necessary on the assignment to prevent a mypy error, as using typing.TYPE_CHECKING alone is insufficient.


if es is not None and es256 is not None: # pragma: NO COVER
__all__ = [
"EsSigner",
"EsVerifier",
"ES256Signer",
"ES256Verifier",
"RSASigner",
"RSAVerifier",
"Signer",
"Verifier",
]

EsSigner = es.EsSigner
EsVerifier = es.EsVerifier
ES256Signer = es256.ES256Signer
ES256Verifier = es256.ES256Verifier
else: # pragma: NO COVER
__all__ = ["RSASigner", "RSAVerifier", "Signer", "Verifier"]


# Aliases to maintain the v1.0.0 interface, as the crypt module was split
Expand Down Expand Up @@ -82,15 +103,3 @@ class to use for verification. This can be used to select different
if verifier.verify(message, signature):
return True
return False


__all__ = [
"EsSigner",
"EsVerifier",
"ES256Signer",
"ES256Verifier",
"RSASigner",
"RSAVerifier",
"Signer",
"Verifier",
]
24 changes: 0 additions & 24 deletions packages/google-auth/google/auth/crypt/_python_rsa.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
from __future__ import absolute_import

import io
import warnings

from pyasn1.codec.der import decoder # type: ignore
from pyasn1_modules import pem # type: ignore
Expand All @@ -40,11 +39,6 @@
_PKCS8_MARKER = ("-----BEGIN PRIVATE KEY-----", "-----END PRIVATE KEY-----")
_PKCS8_SPEC = PrivateKeyInfo()

_warning_msg = (
"The 'rsa' library is deprecated and will be removed in a future release. "
"Please migrate to 'cryptography'."
)


def _bit_list_to_bytes(bit_list):
"""Converts an iterable of 1s and 0s to bytes.
Expand All @@ -70,21 +64,12 @@ def _bit_list_to_bytes(bit_list):
class RSAVerifier(base.Verifier):
"""Verifies RSA cryptographic signatures using public keys.

.. deprecated::
The `rsa` library has been archived. Please migrate to
`cryptography`.

Args:
public_key (rsa.key.PublicKey): The public key used to verify
signatures.
"""

def __init__(self, public_key):
warnings.warn(
_warning_msg,
category=DeprecationWarning,
stacklevel=2,
)
self._pubkey = public_key

@_helpers.copy_docstring(base.Verifier)
Expand Down Expand Up @@ -131,10 +116,6 @@ def from_string(cls, public_key):
class RSASigner(base.Signer, base.FromServiceAccountMixin):
"""Signs messages with an RSA private key.

.. deprecated::
The `rsa` library has been archived. Please migrate to
`cryptography`.

Args:
private_key (rsa.key.PrivateKey): The private key to sign with.
key_id (str): Optional key ID used to identify this private key. This
Expand All @@ -143,11 +124,6 @@ class RSASigner(base.Signer, base.FromServiceAccountMixin):
"""

def __init__(self, private_key, key_id=None):
warnings.warn(
_warning_msg,
category=DeprecationWarning,
stacklevel=2,
)
self._key = private_key
self._key_id = key_id

Expand Down
4 changes: 2 additions & 2 deletions packages/google-auth/google/auth/crypt/es.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,15 +102,15 @@ def verify(self, message: bytes, signature: bytes) -> bool:

@classmethod
def from_string(cls, public_key: Union[str, bytes]) -> "EsVerifier":
"""Construct a Verifier instance from a public key or public
"""Construct an Verifier instance from a public key or public
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Typo in docstring: "an Verifier" should be "a Verifier".

Suggested change
"""Construct an Verifier instance from a public key or public
"""Construct a Verifier instance from a public key or public

certificate string.

Args:
public_key (Union[str, bytes]): The public key in PEM format or the
x509 public key certificate.

Returns:
google.auth.crypt.Verifier: The constructed verifier.
Verifier: The constructed verifier.

Raises:
ValueError: If the public key can't be parsed.
Expand Down
126 changes: 12 additions & 114 deletions packages/google-auth/google/auth/crypt/rsa.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,121 +12,19 @@
# See the License for the specific language governing permissions and
# limitations under the License.

"""
RSA cryptography signer and verifier.
"""RSA cryptography signer and verifier."""

This file provides a shared wrapper, that defers to _python_rsa or _cryptography_rsa
for implmentations using different third party libraries
"""

from cryptography.hazmat.primitives.asymmetric.rsa import RSAPrivateKey
from cryptography.hazmat.primitives.asymmetric.rsa import RSAPublicKey
try:
# Prefer cryptograph-based RSA implementation.
from google.auth.crypt import _cryptography_rsa

from google.auth import _helpers
from google.auth.crypt import _cryptography_rsa
from google.auth.crypt import base
RSASigner = _cryptography_rsa.RSASigner
RSAVerifier = _cryptography_rsa.RSAVerifier
except ImportError: # pragma: NO COVER
# Fallback to pure-python RSA implementation if cryptography is
# unavailable.
from google.auth.crypt import _python_rsa

RSA_KEY_MODULE_PREFIX = "rsa.key"


class RSAVerifier(base.Verifier):
"""Verifies RSA cryptographic signatures using public keys.

Args:
public_key (Union["rsa.key.PublicKey", cryptography.hazmat.primitives.asymmetric.rsa.RSAPublicKey]):
The public key used to verify signatures.
Raises:
ImportError: if called with an rsa.key.PublicKey, when the rsa library is not installed
ValueError: if an unrecognized public key is provided
"""

def __init__(self, public_key):
module_str = public_key.__class__.__module__
if isinstance(public_key, RSAPublicKey):
impl_lib = _cryptography_rsa
elif module_str.startswith(RSA_KEY_MODULE_PREFIX):
from google.auth.crypt import _python_rsa

impl_lib = _python_rsa
else:
raise ValueError(f"unrecognized public key type: {type(public_key)}")
self._impl = impl_lib.RSAVerifier(public_key)

@_helpers.copy_docstring(base.Verifier)
def verify(self, message, signature):
return self._impl.verify(message, signature)

@classmethod
def from_string(cls, public_key):
"""Construct a Verifier instance from a public key or public
certificate string.

Args:
public_key (Union[str, bytes]): The public key in PEM format or the
x509 public key certificate.

Returns:
google.auth.crypt.Verifier: The constructed verifier.

Raises:
ValueError: If the public_key can't be parsed.
"""
instance = cls.__new__(cls)
instance._impl = _cryptography_rsa.RSAVerifier.from_string(public_key)
return instance


class RSASigner(base.Signer, base.FromServiceAccountMixin):
"""Signs messages with an RSA private key.

Args:
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
public key or certificate.

Raises:
ImportError: if called with an rsa.key.PrivateKey, when the rsa library is not installed
ValueError: if an unrecognized public key is provided
"""

def __init__(self, private_key, key_id=None):
module_str = private_key.__class__.__module__
if isinstance(private_key, RSAPrivateKey):
impl_lib = _cryptography_rsa
elif module_str.startswith(RSA_KEY_MODULE_PREFIX):
from google.auth.crypt import _python_rsa

impl_lib = _python_rsa
else:
raise ValueError(f"unrecognized private key type: {type(private_key)}")
self._impl = impl_lib.RSASigner(private_key, key_id=key_id)

@property # type: ignore
@_helpers.copy_docstring(base.Signer)
def key_id(self):
return self._impl.key_id

@_helpers.copy_docstring(base.Signer)
def sign(self, message):
return self._impl.sign(message)

@classmethod
def from_string(cls, key, key_id=None):
"""Construct a Signer instance from a private key in PEM format.

Args:
key (str): Private key in PEM format.
key_id (str): An optional key id used to identify the private key.

Returns:
google.auth.crypt.Signer: The constructed signer.

Raises:
ValueError: If the key cannot be parsed as PKCS#1 or PKCS#8 in
PEM format.
"""
instance = cls.__new__(cls)
instance._impl = _cryptography_rsa.RSASigner.from_string(key, key_id=key_id)
return instance
RSASigner = _python_rsa.RSASigner # type: ignore
RSAVerifier = _python_rsa.RSAVerifier # type: ignore
Comment on lines +18 to +30
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

This change introduces a functional regression. The previous implementation (on the LEFT) was a dynamic wrapper that supported both cryptography and rsa library key objects at runtime. By statically aliasing RSASigner and RSAVerifier to one implementation or the other at import time, the library loses its ability to handle different key types interchangeably if both libraries are present.

Comment on lines +29 to +30
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Use a more specific type ignore like # type: ignore[assignment] when assigning classes in an except ImportError block, consistent with the repository's general rules for optional dependencies.

Suggested change
RSASigner = _python_rsa.RSASigner # type: ignore
RSAVerifier = _python_rsa.RSAVerifier # type: ignore
RSASigner = _python_rsa.RSASigner # type: ignore[assignment]
RSAVerifier = _python_rsa.RSAVerifier # type: ignore[assignment]
References
  1. When handling an optional dependency by assigning None in an except ImportError block, type: ignore[assignment] is necessary on the assignment to prevent a mypy error, as using typing.TYPE_CHECKING alone is insufficient.

22 changes: 4 additions & 18 deletions packages/google-auth/noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
"3.13",
"3.14",
]
ALL_PYTHON = UNIT_TEST_PYTHON_VERSIONS.copy()
ALL_PYTHON = UNIT_TEST_PYTHON_VERSIONS
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Removing .copy() causes ALL_PYTHON to reference the same list as the UNIT_TEST_PYTHON_VERSIONS constant. The subsequent .extend(["3.7"]) call will modify the global constant in-place, which is likely unintended and could affect other sessions.

Suggested change
ALL_PYTHON = UNIT_TEST_PYTHON_VERSIONS
ALL_PYTHON = UNIT_TEST_PYTHON_VERSIONS.copy()

ALL_PYTHON.extend(["3.7"])

# Error if a python version is missing
Expand Down Expand Up @@ -100,7 +100,7 @@ def blacken(session):
@nox.session(python=DEFAULT_PYTHON_VERSION)
def mypy(session):
"""Verify type hints are mypy compatible."""
session.install("-e", ".[aiohttp,rsa]")
session.install("-e", ".[aiohttp]")
session.install(
"mypy",
"types-certifi",
Expand All @@ -115,29 +115,16 @@ def mypy(session):


@nox.session(python=ALL_PYTHON)
@nox.parametrize(["install_deprecated_extras"], (True, False))
def unit(session, install_deprecated_extras):
def unit(session):
# Install all test dependencies, then install this package in-place.

if session.python in ("3.7",):
session.skip("Python 3.7 is no longer supported")
min_py, max_py = UNIT_TEST_PYTHON_VERSIONS[0], UNIT_TEST_PYTHON_VERSIONS[-1]
if not install_deprecated_extras and session.python not in (min_py, max_py):
# only run double tests on first and last supported versions
session.skip(
f"Extended tests only run on boundary Python versions ({min_py}, {max_py}) to reduce CI load."
)

constraints_path = str(
CURRENT_DIRECTORY / "testing" / f"constraints-{session.python}.txt"
)
extras_str = "testing"
if install_deprecated_extras:
# rsa and oauth2client were both archived and support dropped,
# but we still test old code paths
session.install("oauth2client")
extras_str += ",rsa"
session.install("-e", f".[{extras_str}]", "-c", constraints_path)
session.install("-e", ".[testing]", "-c", constraints_path)
session.run(
"pytest",
f"--junitxml=unit_{session.python}_sponge_log.xml",
Expand All @@ -148,7 +135,6 @@ def unit(session, install_deprecated_extras):
"--cov-report=term-missing",
"tests",
"tests_async",
*session.posargs,
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The removal of *session.posargs prevents users from passing additional arguments to pytest from the command line, which is a regression in developer tooling functionality.

Suggested change
)
*session.posargs,
)



Expand Down
Loading
Loading