Skip to content

Commit 8149616

Browse files
committed
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 2795b7a commit 8149616

File tree

4 files changed

+67
-32
lines changed

4 files changed

+67
-32
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: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -45,18 +45,23 @@ 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:
@@ -70,20 +75,34 @@ def identify_required_authn_params(
7075
that were found to satisfy at least one authentication parameter's
7176
requirements or matched one of the `req_authz_tokens`.
7277
"""
73-
required_params: dict[str, list[str]] = {}
78+
required_authn_params: dict[str, list[str]] = {}
7479
used_services: set[str] = set()
7580

81+
# find which of the required authn params are covered by available services.
7682
for param, services in req_authn_params.items():
83+
7784
# if we don't have a token_getter for any of the services required by the param,
7885
# the param is still required
79-
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]
8087

81-
if matched_services:
82-
used_services.update(matched_services)
88+
if matched_authn_services:
89+
used_services.update(matched_authn_services)
8390
else:
84-
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
85104

86-
return required_params, used_services
105+
return required_authn_params, required_authz_tokens, used_services
87106

88107

89108
def params_to_pydantic_model(

packages/toolbox-core/tests/test_utils.py

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,12 @@ def test_create_func_docstring_empty_description():
8383

8484

8585
def test_identify_required_authn_params_none_required():
86-
"""Test when no authentication parameters are required initially."""
87-
req_authn_params = {}
86+
"""Test when no authentication parameters or authorization tokens are required initially."""
87+
req_authn_params: dict[str, list[str]] = {}
88+
req_authz_tokens: list[str] = []
8889
auth_service_names = ["service_a", "service_b"]
89-
expected = {}
90+
expected_params = {}
91+
expected_authz: list[str] = []
9092
expected_used = set()
9193
result = identify_required_authn_params(
9294
req_authn_params, req_authz_tokens, auth_service_names
@@ -99,11 +101,12 @@ def test_identify_required_authn_params_none_required():
99101

100102

101103
def test_identify_required_authn_params_all_covered():
102-
"""Test when all required parameters are covered by available services."""
104+
"""Test when all required authn parameters are covered, no authz tokens."""
103105
req_authn_params = {
104106
"token_a": ["service_a"],
105107
"token_b": ["service_b", "service_c"],
106108
}
109+
req_authz_tokens: list[str] = []
107110
auth_service_names = ["service_a", "service_b"]
108111
expected_params = {}
109112
expected_authz: list[str] = []
@@ -119,15 +122,16 @@ def test_identify_required_authn_params_all_covered():
119122

120123

121124
def test_identify_required_authn_params_some_covered():
122-
"""Test when some parameters are covered, and some are not."""
125+
"""Test when some authn parameters are covered, and some are not, no authz tokens."""
123126
req_authn_params = {
124127
"token_a": ["service_a"],
125128
"token_b": ["service_b", "service_c"],
126129
"token_d": ["service_d"],
127130
"token_e": ["service_e", "service_f"],
128131
}
132+
req_authz_tokens: list[str] = []
129133
auth_service_names = ["service_a", "service_b"]
130-
expected = {
134+
expected_params = {
131135
"token_d": ["service_d"],
132136
"token_e": ["service_e", "service_f"],
133137
}
@@ -145,16 +149,18 @@ def test_identify_required_authn_params_some_covered():
145149

146150

147151
def test_identify_required_authn_params_none_covered():
148-
"""Test when none of the required parameters are covered."""
152+
"""Test when none of the required authn parameters are covered, no authz tokens."""
149153
req_authn_params = {
150154
"token_d": ["service_d"],
151155
"token_e": ["service_e", "service_f"],
152156
}
157+
req_authz_tokens: list[str] = []
153158
auth_service_names = ["service_a", "service_b"]
154-
expected = {
159+
expected_params = {
155160
"token_d": ["service_d"],
156161
"token_e": ["service_e", "service_f"],
157162
}
163+
expected_authz: list[str] = []
158164
expected_used = set()
159165
result = identify_required_authn_params(
160166
req_authn_params, req_authz_tokens, auth_service_names
@@ -167,16 +173,18 @@ def test_identify_required_authn_params_none_covered():
167173

168174

169175
def test_identify_required_authn_params_no_available_services():
170-
"""Test when no authentication services are available."""
176+
"""Test when no authn services are available, no authz tokens."""
171177
req_authn_params = {
172178
"token_a": ["service_a"],
173179
"token_b": ["service_b", "service_c"],
174180
}
175-
auth_service_names = []
176-
expected = {
181+
req_authz_tokens: list[str] = []
182+
auth_service_names: list[str] = []
183+
expected_params = {
177184
"token_a": ["service_a"],
178185
"token_b": ["service_b", "service_c"],
179186
}
187+
expected_authz: list[str] = []
180188
expected_used = set()
181189
result = identify_required_authn_params(
182190
req_authn_params, req_authz_tokens, auth_service_names
@@ -189,14 +197,16 @@ def test_identify_required_authn_params_no_available_services():
189197

190198

191199
def test_identify_required_authn_params_empty_services_for_param():
192-
"""Test edge case where a param requires an empty list of services."""
200+
"""Test edge case: authn param requires an empty list of services, no authz tokens."""
193201
req_authn_params = {
194202
"token_x": [],
195203
}
204+
req_authz_tokens: list[str] = []
196205
auth_service_names = ["service_a"]
197-
expected = {
206+
expected_params = {
198207
"token_x": [],
199208
}
209+
expected_authz: list[str] = []
200210
expected_used = set()
201211
result = identify_required_authn_params(
202212
req_authn_params, req_authz_tokens, auth_service_names

0 commit comments

Comments
 (0)