Skip to content

Commit 5da28e3

Browse files
vvermanvverman
committed
Addressed comments.
1 parent 0d0d96b commit 5da28e3

8 files changed

Lines changed: 162 additions & 38 deletions

File tree

‎packages/google-auth/google/auth/aio/transport/sessions.py‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,13 @@ def __init__(
150150
async def configure_mtls_channel(self, client_cert_callback=None):
151151
"""Configure the client certificate and key for SSL connection.
152152
153-
The function does nothing unless `GOOGLE_API_USE_CLIENT_CERTIFICATE` is
154-
explicitly set to `true`. In this case if client certificate and key are
155-
successfully obtained (from the given client_cert_callback or from application
156-
default SSL credentials), the underlying transport will be reconfigured
157-
to use mTLS.
153+
The function does nothing unless client certificate usage is enabled.
154+
This is true if the `GOOGLE_API_USE_CLIENT_CERTIFICATE` environment
155+
variable is explicitly set to `"true"`, or if the environment variable
156+
is unset/empty but a client certificate configuration is found.
157+
In this case if client certificate and key are successfully obtained (from
158+
the given client_cert_callback or from application default SSL credentials),
159+
the underlying transport will be reconfigured to use mTLS.
158160
Note: This function does nothing if the `aiohttp` library is not
159161
installed.
160162
Important: Calling this method will close any ongoing API requests associated
@@ -215,6 +217,7 @@ async def _do_configure():
215217
ImportError,
216218
OSError,
217219
) as caught_exc:
220+
self._is_mtls = False
218221
new_exc = exceptions.MutualTLSChannelError(caught_exc)
219222
raise new_exc from caught_exc
220223

‎packages/google-auth/google/auth/transport/grpc.py‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -280,14 +280,18 @@ def my_client_cert_callback():
280280
class SslCredentials:
281281
"""Class for application default SSL credentials.
282282
283-
The behavior is controlled by `GOOGLE_API_USE_CLIENT_CERTIFICATE` environment
284-
variable whose default value is `false`. Client certificate will not be used
285-
unless the environment variable is explicitly set to `true`. See
286-
https://google.aip.dev/auth/4114
287-
288-
If the environment variable is `true`, then for devices with endpoint verification
289-
support, a device certificate will be automatically loaded and mutual TLS will
290-
be established.
283+
The client certificate usage (mutual TLS) is determined by the
284+
`should_use_client_cert` helper. Client certificate will not be used
285+
unless client certificate usage is enabled. This is true if the
286+
`GOOGLE_API_USE_CLIENT_CERTIFICATE` environment variable is explicitly
287+
set to `"true"`, or if the environment variable is unset/empty but a client
288+
certificate configuration is found (e.g. via the `GOOGLE_API_CERTIFICATE_CONFIG`
289+
environment variable containing a `"workload"` certificate configuration).
290+
See https://google.aip.dev/auth/4114
291+
292+
If client certificate usage is enabled, then for devices with endpoint
293+
verification support, a device certificate will be automatically loaded and
294+
mutual TLS will be established.
291295
See https://cloud.google.com/endpoint-verification/docs/overview.
292296
"""
293297

@@ -316,11 +320,16 @@ def ssl_credentials(self):
316320
"""
317321
if self._is_mtls:
318322
try:
319-
_, cert, key, _ = _mtls_helper.get_client_ssl_credentials()
320-
self._ssl_credentials = grpc.ssl_channel_credentials(
321-
certificate_chain=cert, private_key=key
322-
)
323-
except exceptions.ClientCertError as caught_exc:
323+
has_cert, cert, key, _ = _mtls_helper.get_client_ssl_credentials()
324+
if has_cert:
325+
self._ssl_credentials = grpc.ssl_channel_credentials(
326+
certificate_chain=cert, private_key=key
327+
)
328+
else:
329+
self._ssl_credentials = grpc.ssl_channel_credentials()
330+
self._is_mtls = False
331+
except (exceptions.ClientCertError, OSError) as caught_exc:
332+
self._is_mtls = False
324333
new_exc = exceptions.MutualTLSChannelError(caught_exc)
325334
raise new_exc from caught_exc
326335
else:

‎packages/google-auth/google/auth/transport/requests.py‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -428,11 +428,13 @@ def __init__(
428428
def configure_mtls_channel(self, client_cert_callback=None):
429429
"""Configure the client certificate and key for SSL connection.
430430
431-
The function does nothing unless `GOOGLE_API_USE_CLIENT_CERTIFICATE` is
432-
explicitly set to `true`. In this case if client certificate and key are
433-
successfully obtained (from the given client_cert_callback or from application
434-
default SSL credentials), a :class:`_MutualTlsAdapter` instance will be mounted
435-
to "https://" prefix.
431+
The function does nothing unless client certificate usage is enabled.
432+
This is true if the `GOOGLE_API_USE_CLIENT_CERTIFICATE` environment
433+
variable is explicitly set to `"true"`, or if the environment variable
434+
is unset/empty but a client certificate configuration is found.
435+
In this case if client certificate and key are successfully obtained (from
436+
the given client_cert_callback or from application default SSL credentials),
437+
a :class:`_MutualTlsAdapter` instance will be mounted to "https://" prefix.
436438
437439
Args:
438440
client_cert_callback (Optional[Callable[[], (bytes, bytes)]]):
@@ -452,6 +454,7 @@ def configure_mtls_channel(self, client_cert_callback=None):
452454
try:
453455
import OpenSSL
454456
except ImportError as caught_exc:
457+
self._is_mtls = False
455458
new_exc = exceptions.MutualTLSChannelError(caught_exc)
456459
raise new_exc from caught_exc
457460

@@ -471,8 +474,10 @@ def configure_mtls_channel(self, client_cert_callback=None):
471474
except (
472475
exceptions.ClientCertError,
473476
ImportError,
477+
OSError,
474478
OpenSSL.crypto.Error,
475479
) as caught_exc:
480+
self._is_mtls = False
476481
new_exc = exceptions.MutualTLSChannelError(caught_exc)
477482
raise new_exc from caught_exc
478483

‎packages/google-auth/google/auth/transport/urllib3.py‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -313,13 +313,15 @@ def __init__(
313313

314314
def configure_mtls_channel(self, client_cert_callback=None):
315315
"""Configures mutual TLS channel using the given client_cert_callback or
316-
application default SSL credentials. The behavior is controlled by
317-
`GOOGLE_API_USE_CLIENT_CERTIFICATE` environment variable.
318-
(1) If the environment variable value is `true`, the function returns True
319-
if the channel is mutual TLS and False otherwise. The `http` provided
320-
in the constructor will be overwritten.
321-
(2) If the environment variable is not set or `false`, the function does
322-
nothing and it always return False.
316+
application default SSL credentials. The behavior is determined by the
317+
`should_use_client_cert` helper.
318+
(1) If client certificate usage is enabled (the environment variable
319+
`GOOGLE_API_USE_CLIENT_CERTIFICATE` is explicitly set to `"true"`, or if
320+
the environment variable is unset/empty but a client certificate configuration
321+
is found), the function returns True if the channel is mutual TLS and False
322+
otherwise. The `http` provided in the constructor will be overwritten.
323+
(2) If client certificate usage is disabled, the function does nothing and
324+
always returns False.
323325
324326
Args:
325327
client_cert_callback (Optional[Callable[[], (bytes, bytes)]]):
@@ -344,6 +346,7 @@ def configure_mtls_channel(self, client_cert_callback=None):
344346
try:
345347
import OpenSSL
346348
except ImportError as caught_exc:
349+
self._is_mtls = False
347350
new_exc = exceptions.MutualTLSChannelError(caught_exc)
348351
raise new_exc from caught_exc
349352

@@ -361,8 +364,10 @@ def configure_mtls_channel(self, client_cert_callback=None):
361364
except (
362365
exceptions.ClientCertError,
363366
ImportError,
367+
OSError,
364368
OpenSSL.crypto.Error,
365369
) as caught_exc:
370+
self._is_mtls = False
366371
new_exc = exceptions.MutualTLSChannelError(caught_exc)
367372
raise new_exc from caught_exc
368373

‎packages/google-auth/tests/transport/aio/test_sessions_mtls.py‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,3 +177,30 @@ async def test_configure_mtls_channel_custom_request(self):
177177
mock_make_context.assert_called_once_with(
178178
b"fake_cert_data", b"fake_key_data"
179179
)
180+
181+
@pytest.mark.asyncio
182+
async def test_configure_mtls_channel_exception_resets_flag(self):
183+
"""
184+
Tests that self._is_mtls is reset to False if an exception is raised
185+
during configuration.
186+
"""
187+
with mock.patch.dict(
188+
os.environ, {"GOOGLE_API_USE_CLIENT_CERTIFICATE": "true"}
189+
), mock.patch("os.path.exists") as mock_exists, mock.patch(
190+
"builtins.open", mock.mock_open(read_data=json.dumps(VALID_WORKLOAD_CONFIG))
191+
), mock.patch(
192+
"google.auth.aio.transport.mtls.get_client_cert_and_key"
193+
) as mock_helper, mock.patch(
194+
"google.auth.aio.transport.mtls.make_client_cert_ssl_context"
195+
) as mock_make_context:
196+
mock_exists.return_value = True
197+
mock_helper.return_value = (True, b"fake_cert_data", b"fake_key_data")
198+
mock_make_context.side_effect = exceptions.ClientCertError("Mock error")
199+
200+
mock_creds = mock.AsyncMock(spec=credentials.Credentials)
201+
session = sessions.AsyncAuthorizedSession(mock_creds)
202+
203+
with pytest.raises(exceptions.MutualTLSChannelError):
204+
await session.configure_mtls_channel()
205+
206+
assert session._is_mtls is False

‎packages/google-auth/tests/transport/test_grpc.py‎

Lines changed: 63 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -216,9 +216,12 @@ def test_secure_authorized_channel_adc_without_client_cert_env(
216216
request = mock.create_autospec(transport.Request)
217217
target = "example.com:80"
218218

219-
channel = google.auth.transport.grpc.secure_authorized_channel(
220-
credentials, request, target, options=mock.sentinel.options
221-
)
219+
with mock.patch.dict(
220+
os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "false"}
221+
):
222+
channel = google.auth.transport.grpc.secure_authorized_channel(
223+
credentials, request, target, options=mock.sentinel.options
224+
)
222225

223226
# Check the auth plugin construction.
224227
auth_plugin = metadata_call_credentials.call_args[0][0]
@@ -375,9 +378,12 @@ def test_secure_authorized_channel_cert_callback_without_client_cert_env(
375378
target = "example.com:80"
376379
client_cert_callback = mock.Mock()
377380

378-
google.auth.transport.grpc.secure_authorized_channel(
379-
credentials, request, target, client_cert_callback=client_cert_callback
380-
)
381+
with mock.patch.dict(
382+
os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "false"}
383+
):
384+
google.auth.transport.grpc.secure_authorized_channel(
385+
credentials, request, target, client_cert_callback=client_cert_callback
386+
)
381387

382388
# Check client_cert_callback is not called because GOOGLE_API_USE_CLIENT_CERTIFICATE
383389
# is not set.
@@ -510,12 +516,61 @@ def test_get_client_ssl_credentials_without_client_cert_env(
510516
mock_get_client_ssl_credentials,
511517
mock_ssl_channel_credentials,
512518
):
513-
# Test client cert won't be used if GOOGLE_API_USE_CLIENT_CERTIFICATE is not set.
514-
ssl_credentials = google.auth.transport.grpc.SslCredentials()
519+
with mock.patch.dict(
520+
os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "false"}
521+
):
522+
ssl_credentials = google.auth.transport.grpc.SslCredentials()
515523

516524
assert ssl_credentials.ssl_credentials is not None
517525
assert not ssl_credentials.is_mtls
518526
mock_check_config_path.assert_not_called()
519527
mock_load_json_file.assert_not_called()
520528
mock_get_client_ssl_credentials.assert_not_called()
521529
mock_ssl_channel_credentials.assert_called_once()
530+
531+
def test_get_client_ssl_credentials_no_workload_cert(
532+
self,
533+
mock_check_config_path,
534+
mock_load_json_file,
535+
mock_get_client_ssl_credentials,
536+
mock_ssl_channel_credentials,
537+
):
538+
mock_check_config_path.return_value = METADATA_PATH
539+
mock_load_json_file.return_value = {"cert_provider_command": ["some command"]}
540+
mock_get_client_ssl_credentials.return_value = (
541+
False,
542+
None,
543+
None,
544+
None,
545+
)
546+
547+
with mock.patch.dict(
548+
os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"}
549+
):
550+
ssl_credentials = google.auth.transport.grpc.SslCredentials()
551+
552+
assert ssl_credentials.ssl_credentials is not None
553+
assert not ssl_credentials.is_mtls
554+
mock_get_client_ssl_credentials.assert_called_once()
555+
mock_ssl_channel_credentials.assert_called_once_with()
556+
557+
def test_get_client_ssl_credentials_os_error(
558+
self,
559+
mock_check_config_path,
560+
mock_load_json_file,
561+
mock_get_client_ssl_credentials,
562+
mock_ssl_channel_credentials,
563+
):
564+
mock_check_config_path.return_value = METADATA_PATH
565+
mock_load_json_file.return_value = {"cert_provider_command": ["some command"]}
566+
mock_get_client_ssl_credentials.side_effect = OSError("Mock file read error")
567+
568+
with mock.patch.dict(
569+
os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"}
570+
):
571+
ssl_credentials = google.auth.transport.grpc.SslCredentials()
572+
573+
with pytest.raises(exceptions.MutualTLSChannelError):
574+
_ = ssl_credentials.ssl_credentials
575+
576+
assert not ssl_credentials.is_mtls

‎packages/google-auth/tests/transport/test_requests.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,15 @@ def test_configure_mtls_channel_exceptions(self, mock_get_client_cert_and_key):
490490
os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"}
491491
):
492492
auth_session.configure_mtls_channel()
493+
assert auth_session._is_mtls is False
494+
495+
mock_get_client_cert_and_key.side_effect = OSError("Mock file read error")
496+
with pytest.raises(exceptions.MutualTLSChannelError):
497+
with mock.patch.dict(
498+
os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"}
499+
):
500+
auth_session.configure_mtls_channel()
501+
assert auth_session._is_mtls is False
493502

494503
mock_get_client_cert_and_key.return_value = (False, None, None)
495504
with mock.patch.dict("sys.modules"):
@@ -500,6 +509,7 @@ def test_configure_mtls_channel_exceptions(self, mock_get_client_cert_and_key):
500509
{environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"},
501510
):
502511
auth_session.configure_mtls_channel()
512+
assert auth_session._is_mtls is False
503513

504514
@mock.patch(
505515
"google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True

‎packages/google-auth/tests/transport/test_urllib3.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,15 @@ def test_configure_mtls_channel_exceptions(self, mock_get_client_cert_and_key):
282282
os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"}
283283
):
284284
authed_http.configure_mtls_channel()
285+
assert authed_http._is_mtls is False
286+
287+
mock_get_client_cert_and_key.side_effect = OSError("Mock file read error")
288+
with pytest.raises(exceptions.MutualTLSChannelError):
289+
with mock.patch.dict(
290+
os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"}
291+
):
292+
authed_http.configure_mtls_channel()
293+
assert authed_http._is_mtls is False
285294

286295
mock_get_client_cert_and_key.return_value = (False, None, None)
287296
with mock.patch.dict("sys.modules"):
@@ -292,6 +301,7 @@ def test_configure_mtls_channel_exceptions(self, mock_get_client_cert_and_key):
292301
{environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"},
293302
):
294303
authed_http.configure_mtls_channel()
304+
assert authed_http._is_mtls is False
295305

296306
@mock.patch(
297307
"google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True

0 commit comments

Comments
 (0)