Skip to content

Commit fdfa4b1

Browse files
GWealecopybara-github
authored andcommitted
fix(workflow): omit the oauth client secret from the credential request event
Co-authored-by: George Weale <gweale@google.com> PiperOrigin-RevId: 974138172
1 parent f7c0c01 commit fdfa4b1

2 files changed

Lines changed: 74 additions & 5 deletions

File tree

src/google/adk/workflow/utils/_workflow_hitl_utils.py

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,27 @@ def _build_auth_message(auth_config: AuthConfig) -> str:
163163
return 'Please provide your authentication credentials.'
164164

165165

166+
def _without_client_secret(auth_config: AuthConfig) -> AuthConfig:
167+
"""Returns a copy of the auth config with the OAuth client secret removed.
168+
169+
The auth request is handed to the caller, which needs the authorization uri
170+
and the state to complete the flow but never the developer's client secret.
171+
The secret stays on this side and is supplied again when the response comes
172+
back.
173+
174+
Args:
175+
auth_config: The auth configuration for the node.
176+
"""
177+
without_secret = auth_config.model_copy(deep=True)
178+
for credential in (
179+
without_secret.raw_auth_credential,
180+
without_secret.exchanged_auth_credential,
181+
):
182+
if credential and credential.oauth2:
183+
credential.oauth2.client_secret = None
184+
return without_secret
185+
186+
166187
def create_auth_request_event(
167188
auth_config: AuthConfig,
168189
interrupt_id: str,
@@ -190,7 +211,7 @@ def create_auth_request_event(
190211
state[_oauth_state_key(interrupt_id)] = generated_credential.oauth2.state
191212
args = AuthToolArguments(
192213
function_call_id=interrupt_id,
193-
auth_config=auth_request,
214+
auth_config=_without_client_secret(auth_request),
194215
).model_dump(mode='json', exclude_none=True, by_alias=True)
195216

196217
# Add message so the UI / CLI knows what to display.
@@ -291,6 +312,23 @@ async def process_auth_resume(
291312

292313
resumed_config = auth_config.model_copy(deep=True)
293314
resumed_config.exchanged_auth_credential = exchanged_credential
315+
316+
# The client secret is never handed out with the request, so the response
317+
# cannot carry it back; the node's own credential supplies it for the token
318+
# exchange.
319+
raw_oauth2 = (
320+
resumed_config.raw_auth_credential.oauth2
321+
if resumed_config.raw_auth_credential
322+
else None
323+
)
324+
exchanged_oauth2 = (
325+
resumed_config.exchanged_auth_credential.oauth2
326+
if resumed_config.exchanged_auth_credential
327+
else None
328+
)
329+
if raw_oauth2 and raw_oauth2.client_secret and exchanged_oauth2:
330+
exchanged_oauth2.client_secret = raw_oauth2.client_secret
331+
294332
await AuthHandler(auth_config=resumed_config).parse_and_store_auth_response(
295333
state=state
296334
)

tests/unittests/workflow/utils/test_workflow_hitl_utils.py

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,18 @@ def test_args_are_json_serializable(self):
218218
json.dumps(fc.args)
219219
assert fc.args["authConfig"]["authScheme"]["type"] == "oauth2"
220220

221+
def test_client_secret_is_not_handed_to_the_caller(self):
222+
"""The caller gets what completes the flow; the secret stays behind."""
223+
auth_config = _oauth_auth_config()
224+
event = create_auth_request_event(auth_config, "auth-id-1", _empty_state())
225+
226+
fc = event.content.parts[0].function_call
227+
oauth2 = fc.args["authConfig"]["exchangedAuthCredential"]["oauth2"]
228+
assert oauth2["authUri"]
229+
assert oauth2["state"]
230+
assert oauth2["clientId"] == "client-id"
231+
assert "client-secret" not in json.dumps(fc.args)
232+
221233

222234
# --- process_auth_resume / has_auth_credential ---
223235

@@ -363,7 +375,6 @@ def _oauth_resume_response(auth_config, state_value: str):
363375
auth_type=AuthCredentialTypes.OAUTH2,
364376
oauth2=OAuth2Auth(
365377
client_id="client-id",
366-
client_secret="client-secret",
367378
state=state_value,
368379
auth_code="authorization-code",
369380
),
@@ -381,17 +392,20 @@ class TestProcessAuthResumeOAuth:
381392

382393
@pytest.fixture(autouse=True)
383394
def _no_network_exchange(self, monkeypatch):
384-
"""Records the auth scheme each exchange runs against, without network."""
395+
"""Records what each exchange runs against, without network."""
385396
from google.adk.auth import auth_handler as auth_handler_module
386397
from google.adk.auth.exchanger.base_credential_exchanger import ExchangeResult
387398

388399
self.exchanged_schemes = []
389-
recorded = self.exchanged_schemes
400+
self.exchanged_credentials = []
401+
recorded_schemes = self.exchanged_schemes
402+
recorded_credentials = self.exchanged_credentials
390403

391404
class _RecordingExchanger:
392405

393406
async def exchange(self, auth_credential, auth_scheme=None):
394-
recorded.append(auth_scheme)
407+
recorded_schemes.append(auth_scheme)
408+
recorded_credentials.append(auth_credential)
395409
return ExchangeResult(auth_credential, True)
396410

397411
monkeypatch.setattr(
@@ -433,6 +447,23 @@ async def test_existence_check_does_not_exchange(self):
433447
assert has_auth_credential(auth_config, state) is True
434448
assert len(self.exchanged_schemes) == exchanges_so_far
435449

450+
@pytest.mark.asyncio
451+
async def test_exchange_uses_the_client_secret_from_the_node(self):
452+
"""The response has no secret to echo, so the node's config supplies it."""
453+
auth_config = _oauth_auth_config()
454+
state = _empty_state()
455+
event = create_auth_request_event(auth_config, "auth-id-1", state)
456+
457+
await process_auth_resume(
458+
_oauth_resume_response(auth_config, _requested_state(event)),
459+
auth_config,
460+
state,
461+
"auth-id-1",
462+
)
463+
464+
assert len(self.exchanged_credentials) == 1
465+
assert self.exchanged_credentials[0].oauth2.client_secret == "client-secret"
466+
436467
@pytest.mark.asyncio
437468
async def test_response_with_another_state_is_rejected(self):
438469
"""A response that does not echo the generated state is not exchanged."""

0 commit comments

Comments
 (0)