fix: Add validation to ensure added auth token getters are used by the tool #220
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
This PR fixes an issue where the system could inaccurately identify the authorization tokens still needed for tool invocation. The `identify_required_authn_params` helper has been updated to leverage its new capability of recognizing all alternatives of required authorization tokens. A new `ToolboxTool` member variable, `__required_authz_tokens`, now stores these alternatives. The tool invocation logic correctly uses this to check if any matching token has been provided. This new member variable is also updated correctly by the remaining authz tokens while adding auth token getters, and validated right before tool invocation.
anubhav756
force-pushed
the
anubhav-tool-unused-auth
branch
from
74ddda7 to
86f44f5
Compare
…e tool Previously, the `ToolboxTool.add_auth_token_getters` method only validated against existing registered getters or conflicts with client headers. It did not verify if *all* the auth token getters provided were actually used or required by the specific tool instance they were being added to. This PR enhances the validation in `add_auth_token_getters`. It now leverages the `used_auth_token_getters` information returned by the existing `identify_required_authn_params` call. This allows the method to confirm that every getter passed in is genuinely required by the tool, raising an error if any are unused. This ensures that only relevant auth token getters are attempted to be registered for a tool, preventing misconfigurations and human errors. > [!NOTE] > This validation aligns with the existing validation logic already present in the `ToolboxClient.load_tool` method, promoting a consistent approach to handling auth token getter requirements across the codebase.
anubhav756
force-pushed
the
anubhav-tool-unused-auth
branch
from
06847ff to
4405195
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Previously, the
ToolboxTool.add_auth_token_gettersmethod only validated against existing registered getters or conflicts with client headers. It did not verify if all the auth token getters provided were actually used or required by the specific tool instance they were being added to.Solution
This PR enhances the validation in
add_auth_token_getters. It now leverages theused_auth_token_gettersinformation returned by the existingidentify_required_authn_paramscall. This allows the method to confirm that every getter passed in is genuinely required by the tool, raising an error if any are unused.Benefit
This ensures that only relevant auth token getters are attempted to be registered for a tool, preventing misconfigurations and human errors.
Note
This validation aligns with the existing validation logic already present in the
ToolboxClient.load_toolmethod, promoting a consistent approach to handling auth token getter requirements across the codebase.