Skip to content

Commit 5d49936

Browse files
authored
feat: Introduce identifying used authz token getters (#221)
* chore: Add unit test coverage. * chore: Consolidate auth header creation logic Post adding the feature of adding client-level auth headers (#178), we have the logic for creating an auth header, from the given auth token getter name, in 3 different places. This PR unifies all of that logic into a single helper to improve maintenance, and make it easier to change the way we add suffix/prefix, and reduces WET code. * docs: Add names to return values for better readability * chore: Improve readability * chore: Remove redundant tests * feat: Introduce identifying used authz token getters This PR adds the feature in `identify_required_authn_params` helper to determine which of the provided auth token getters are actively used to satisfy a tool's authorization requirements (as defined by the `authRequired` key in its manifest). This is a foundational step towards future validation in `ToolboxTool.add_auth_token_getters`, which will ensure no configured auth token getters remain unused, thereby preventing potential misconfigurations.
1 parent cdfaaf8 commit 5d49936

File tree

5 files changed

+271
-53
lines changed

5 files changed

+271
-53
lines changed

packages/toolbox-core/src/toolbox_core/client.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,11 @@ def __parse_tool(
7979
else: # regular parameter
8080
params.append(p)
8181

82-
authn_params, used_auth_keys = identify_required_authn_params(
83-
authn_params, auth_token_getters.keys()
82+
authn_params, _, used_auth_keys = identify_required_authn_params(
83+
# TODO: Add schema.authRequired as second arg
84+
authn_params,
85+
[],
86+
auth_token_getters.keys(),
8487
)
8588

8689
tool = ToolboxTool(

packages/toolbox-core/src/toolbox_core/tool.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,10 @@ def add_auth_token_getters(
299299
# create a read-only updated for params that are still required
300300
new_req_authn_params = MappingProxyType(
301301
identify_required_authn_params(
302-
self.__required_authn_params, auth_token_getters.keys()
302+
# TODO: Add authRequired
303+
self.__required_authn_params,
304+
[],
305+
auth_token_getters.keys(),
303306
)[0]
304307
)
305308

packages/toolbox-core/src/toolbox_core/utils.py

Lines changed: 44 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -45,41 +45,64 @@ def create_func_docstring(description: str, params: Sequence[ParameterSchema]) -
4545

4646

4747
def identify_required_authn_params(
48-
req_authn_params: Mapping[str, list[str]], auth_service_names: Iterable[str]
49-
) -> tuple[dict[str, list[str]], set[str]]:
48+
req_authn_params: Mapping[str, list[str]],
49+
req_authz_tokens: list[str],
50+
auth_service_names: Iterable[str],
51+
) -> tuple[dict[str, list[str]], list[str], set[str]]:
5052
"""
51-
Identifies authentication parameters that are still required; because they
52-
are not covered by the provided `auth_service_names`, and also returns a
53-
set of all authentication services that were found to be matching.
53+
Identifies authentication parameters and authorization tokens that are still
54+
required because they are not covered by the provided `auth_service_names`.
55+
Also returns a set of all authentication/authorization services from
56+
`auth_service_names` that were found to be matching.
5457
55-
Args:
56-
req_authn_params: A mapping of parameter names to lists of required
57-
authentication services.
58-
auth_service_names: An iterable of authentication service names for which
59-
token getters are available.
58+
Args:
59+
req_authn_params: A mapping of parameter names to lists of required
60+
authentication services for those parameters.
61+
req_authz_tokens: A list of strings representing all authorization
62+
tokens that are required to invoke the current tool.
63+
auth_service_names: An iterable of authentication/authorization service
64+
names for which token getters are available.
6065
6166
Returns:
6267
A tuple containing:
63-
- A new dictionary representing the subset of required
64-
authentication parameters that are not covered by the provided
65-
`auth_service_names`.
66-
- A list of authentication service names from `auth_service_names`
67-
that were found to satisfy at least one parameter's requirements.
68+
- required_authn_params: A new dictionary representing the subset of
69+
required authentication parameters that are not covered by the
70+
provided `auth_service_names`.
71+
- required_authz_tokens: A list of required authorization tokens if
72+
no service name in `auth_service_names` matches any token in
73+
`req_authz_tokens`. If any match is found, this list is empty.
74+
- used_services: A set of service names from `auth_service_names`
75+
that were found to satisfy at least one authentication parameter's
76+
requirements or matched one of the `req_authz_tokens`.
6877
"""
69-
required_params: dict[str, list[str]] = {}
78+
required_authn_params: dict[str, list[str]] = {}
7079
used_services: set[str] = set()
7180

81+
# find which of the required authn params are covered by available services.
7282
for param, services in req_authn_params.items():
83+
7384
# if we don't have a token_getter for any of the services required by the param,
7485
# the param is still required
75-
matched_services = [s for s in services if s in auth_service_names]
86+
matched_authn_services = [s for s in services if s in auth_service_names]
7687

77-
if matched_services:
78-
used_services.update(matched_services)
88+
if matched_authn_services:
89+
used_services.update(matched_authn_services)
7990
else:
80-
required_params[param] = services
91+
required_authn_params[param] = services
92+
93+
# find which of the required authz tokens are covered by available services.
94+
matched_authz_services = [s for s in auth_service_names if s in req_authz_tokens]
95+
required_authz_tokens: list[str] = []
96+
97+
# If a match is found, authorization is met (no remaining required tokens).
98+
# Otherwise, all `req_authz_tokens` are still required. (Handles empty
99+
# `req_authz_tokens` correctly, resulting in no required tokens).
100+
if matched_authz_services:
101+
used_services.update(matched_authz_services)
102+
else:
103+
required_authz_tokens = req_authz_tokens
81104

82-
return required_params, used_services
105+
return required_authn_params, required_authz_tokens, used_services
83106

84107

85108
def params_to_pydantic_model(

packages/toolbox-core/tests/test_client.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -607,7 +607,7 @@ async def test_bind_param_fail(self, tool_name, client):
607607
assert len(tool.__signature__.parameters) == 2
608608
assert "argA" in tool.__signature__.parameters
609609

610-
with pytest.raises(ValueError) as e:
610+
with pytest.raises(Exception) as e:
611611
tool.bind_param("argC", lambda: 5)
612612
assert "unable to bind parameters: no parameter named argC" in str(e.value)
613613

0 commit comments

Comments
 (0)