From a81a4d2b30245431014d4274d1e5f31f109daafe Mon Sep 17 00:00:00 2001 From: Anubhav Dhawan Date: Tue, 6 May 2025 17:11:47 +0530 Subject: [PATCH 1/6] chore: Add unit test cases --- packages/toolbox-core/tests/test_client.py | 29 ++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/packages/toolbox-core/tests/test_client.py b/packages/toolbox-core/tests/test_client.py index 1b36fe0d..1a4860d3 100644 --- a/packages/toolbox-core/tests/test_client.py +++ b/packages/toolbox-core/tests/test_client.py @@ -398,6 +398,35 @@ async def test_constructor_getters_missing_fail(self, tool_name, client): await client.load_tool(tool_name, auth_token_getters={AUTH_SERVICE: {}}) + @pytest.mark.asyncio + async def test_add_auth_token_getters_missing_fail(self, tool_name, client): + """ + Tests that adding a missing auth token getter raises ValueError. + """ + AUTH_SERVICE = "xmy-auth-service" + + tool = await client.load_tool(tool_name) + + with pytest.raises( + ValueError, + match=f"Authentication source\(s\) \`{AUTH_SERVICE}\` unused by tool \`{tool_name}\`.", + ): + tool.add_auth_token_getters({AUTH_SERVICE: {}}) + + @pytest.mark.asyncio + async def test_constructor_getters_missing_fail(self, tool_name, client): + """ + Tests that adding a missing auth token getter raises ValueError. + """ + AUTH_SERVICE = "xmy-auth-service" + + with pytest.raises( + ValueError, + match=f"Validation failed for tool '{tool_name}': unused auth tokens: {AUTH_SERVICE}.", + ): + await client.load_tool(tool_name, auth_token_getters={AUTH_SERVICE: {}}) + + class TestBoundParameter: @pytest.fixture From 3090b77dcf31820e63dc3629892962001bebfee1 Mon Sep 17 00:00:00 2001 From: Anubhav Dhawan Date: Tue, 6 May 2025 17:12:37 +0530 Subject: [PATCH 2/6] chore: Delint --- packages/toolbox-core/tests/test_client.py | 29 ---------------------- 1 file changed, 29 deletions(-) diff --git a/packages/toolbox-core/tests/test_client.py b/packages/toolbox-core/tests/test_client.py index 1a4860d3..1b36fe0d 100644 --- a/packages/toolbox-core/tests/test_client.py +++ b/packages/toolbox-core/tests/test_client.py @@ -398,35 +398,6 @@ async def test_constructor_getters_missing_fail(self, tool_name, client): await client.load_tool(tool_name, auth_token_getters={AUTH_SERVICE: {}}) - @pytest.mark.asyncio - async def test_add_auth_token_getters_missing_fail(self, tool_name, client): - """ - Tests that adding a missing auth token getter raises ValueError. - """ - AUTH_SERVICE = "xmy-auth-service" - - tool = await client.load_tool(tool_name) - - with pytest.raises( - ValueError, - match=f"Authentication source\(s\) \`{AUTH_SERVICE}\` unused by tool \`{tool_name}\`.", - ): - tool.add_auth_token_getters({AUTH_SERVICE: {}}) - - @pytest.mark.asyncio - async def test_constructor_getters_missing_fail(self, tool_name, client): - """ - Tests that adding a missing auth token getter raises ValueError. - """ - AUTH_SERVICE = "xmy-auth-service" - - with pytest.raises( - ValueError, - match=f"Validation failed for tool '{tool_name}': unused auth tokens: {AUTH_SERVICE}.", - ): - await client.load_tool(tool_name, auth_token_getters={AUTH_SERVICE: {}}) - - class TestBoundParameter: @pytest.fixture From a0c77db31f36d5fc0094b368cb5b0001f594ce85 Mon Sep 17 00:00:00 2001 From: Anubhav Dhawan Date: Tue, 6 May 2025 17:40:02 +0530 Subject: [PATCH 3/6] feat: Warn on insecure tool invocation with authentication This change introduces a warning that is displayed immediately before a tool invocation if: 1. The invocation includes an authentication header. 2. The connection is being made over non-secure HTTP. > [!IMPORTANT] The purpose of this warning is to alert the user to the security risk of sending credentials over an unencrypted channel and to encourage the use of HTTPS. --- packages/toolbox-core/src/toolbox_core/tool.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/toolbox-core/src/toolbox_core/tool.py b/packages/toolbox-core/src/toolbox_core/tool.py index 98d04e17..79da97a8 100644 --- a/packages/toolbox-core/src/toolbox_core/tool.py +++ b/packages/toolbox-core/src/toolbox_core/tool.py @@ -257,18 +257,27 @@ async def __call__(self, *args: Any, **kwargs: Any) -> str: payload[param] = await resolve_value(value) # create headers for auth services - headers = {} + auth_headers = {} for auth_service, token_getter in self.__auth_service_token_getters.items(): - headers[self.__get_auth_header(auth_service)] = await resolve_value( + auth_headers[self.__get_auth_header(auth_service)] = await resolve_value( token_getter ) for client_header_name, client_header_val in self.__client_headers.items(): - headers[client_header_name] = await resolve_value(client_header_val) + auth_headers[client_header_name] = await resolve_value(client_header_val) + + # ID tokens contain sensitive user information (claims). Transmitting + # these over HTTP exposes the data to interception and unauthorized + # access. Always use HTTPS to ensure secure communication and protect + # user privacy. + if auth_headers and not self.__url.startswith("https://"): + warn( + "Sending ID token over HTTP. User data may be exposed. Use HTTPS for secure communication." + ) async with self.__session.post( self.__url, json=payload, - headers=headers, + headers=auth_headers, ) as resp: body = await resp.json() if resp.status < 200 or resp.status >= 300: From e7013607b4ac732c951cd9942dbbc915a8107415 Mon Sep 17 00:00:00 2001 From: Anubhav Dhawan Date: Tue, 13 May 2025 15:57:46 +0530 Subject: [PATCH 4/6] fix!: Warn about https only during tool initialization --- packages/toolbox-core/src/toolbox_core/tool.py | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/packages/toolbox-core/src/toolbox_core/tool.py b/packages/toolbox-core/src/toolbox_core/tool.py index 79da97a8..98d04e17 100644 --- a/packages/toolbox-core/src/toolbox_core/tool.py +++ b/packages/toolbox-core/src/toolbox_core/tool.py @@ -257,27 +257,18 @@ async def __call__(self, *args: Any, **kwargs: Any) -> str: payload[param] = await resolve_value(value) # create headers for auth services - auth_headers = {} + headers = {} for auth_service, token_getter in self.__auth_service_token_getters.items(): - auth_headers[self.__get_auth_header(auth_service)] = await resolve_value( + headers[self.__get_auth_header(auth_service)] = await resolve_value( token_getter ) for client_header_name, client_header_val in self.__client_headers.items(): - auth_headers[client_header_name] = await resolve_value(client_header_val) - - # ID tokens contain sensitive user information (claims). Transmitting - # these over HTTP exposes the data to interception and unauthorized - # access. Always use HTTPS to ensure secure communication and protect - # user privacy. - if auth_headers and not self.__url.startswith("https://"): - warn( - "Sending ID token over HTTP. User data may be exposed. Use HTTPS for secure communication." - ) + headers[client_header_name] = await resolve_value(client_header_val) async with self.__session.post( self.__url, json=payload, - headers=auth_headers, + headers=headers, ) as resp: body = await resp.json() if resp.status < 200 or resp.status >= 300: From 76de17acd1abffd6f8c613133b706e576eed9743 Mon Sep 17 00:00:00 2001 From: Anubhav Dhawan Date: Tue, 13 May 2025 02:52:16 +0530 Subject: [PATCH 5/6] fix!: Make unauth call error message as PermissionError --- packages/toolbox-core/src/toolbox_core/tool.py | 2 +- packages/toolbox-core/tests/test_e2e.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/toolbox-core/src/toolbox_core/tool.py b/packages/toolbox-core/src/toolbox_core/tool.py index 98d04e17..5a9bca6e 100644 --- a/packages/toolbox-core/src/toolbox_core/tool.py +++ b/packages/toolbox-core/src/toolbox_core/tool.py @@ -239,7 +239,7 @@ async def __call__(self, *args: Any, **kwargs: Any) -> str: for s in self.__required_authn_params.values(): req_auth_services.update(s) req_auth_services.update(self.__required_authz_tokens) - raise ValueError( + raise PermissionError( f"One or more of the following authn services are required to invoke this tool" f": {','.join(req_auth_services)}" ) diff --git a/packages/toolbox-core/tests/test_e2e.py b/packages/toolbox-core/tests/test_e2e.py index c8111b6f..8920bc3b 100644 --- a/packages/toolbox-core/tests/test_e2e.py +++ b/packages/toolbox-core/tests/test_e2e.py @@ -147,7 +147,7 @@ async def test_run_tool_no_auth(self, toolbox: ToolboxClient): """Tests running a tool requiring auth without providing auth.""" tool = await toolbox.load_tool("get-row-by-id-auth") with pytest.raises( - Exception, + PermissionError, match="One or more of the following authn services are required to invoke this tool: my-test-auth", ): await tool(id="2") @@ -188,7 +188,7 @@ async def test_run_tool_param_auth_no_auth(self, toolbox: ToolboxClient): """Tests running a tool with a param requiring auth, without auth.""" tool = await toolbox.load_tool("get-row-by-email-auth") with pytest.raises( - ValueError, + PermissionError, match="One or more of the following authn services are required to invoke this tool: my-test-auth", ): await tool() From c6e67f50585d47346a21fd45d9c92a2cc5a3b505 Mon Sep 17 00:00:00 2001 From: Anubhav Dhawan Date: Tue, 13 May 2025 17:12:06 +0530 Subject: [PATCH 6/6] chore: Fix tests --- packages/toolbox-core/tests/test_sync_client.py | 2 +- packages/toolbox-core/tests/test_sync_e2e.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/toolbox-core/tests/test_sync_client.py b/packages/toolbox-core/tests/test_sync_client.py index 4b43e466..28f2b27b 100644 --- a/packages/toolbox-core/tests/test_sync_client.py +++ b/packages/toolbox-core/tests/test_sync_client.py @@ -541,7 +541,7 @@ def test_auth_with_load_tool_fail_no_token( tool = sync_client.load_tool(tool_name_auth) with pytest.raises( - ValueError, + PermissionError, match="One or more of the following authn services are required to invoke this tool: my-auth-service", ): tool(argA=15, argB=True) diff --git a/packages/toolbox-core/tests/test_sync_e2e.py b/packages/toolbox-core/tests/test_sync_e2e.py index 885724e9..f2730e47 100644 --- a/packages/toolbox-core/tests/test_sync_e2e.py +++ b/packages/toolbox-core/tests/test_sync_e2e.py @@ -129,7 +129,7 @@ def test_run_tool_no_auth(self, toolbox: ToolboxSyncClient): """Tests running a tool requiring auth without providing auth.""" tool = toolbox.load_tool("get-row-by-id-auth") with pytest.raises( - Exception, + PermissionError, match="One or more of the following authn services are required to invoke this tool: my-test-auth", ): tool(id="2") @@ -156,7 +156,7 @@ def test_run_tool_param_auth_no_auth(self, toolbox: ToolboxSyncClient): """Tests running a tool with a param requiring auth, without auth.""" tool = toolbox.load_tool("get-row-by-email-auth") with pytest.raises( - ValueError, + PermissionError, match="One or more of the following authn services are required to invoke this tool: my-test-auth", ): tool()