Skip to content

feat(sys): add auth method discovery and detail tools#87

Open
czembower wants to merge 2 commits into
hashicorp:mainfrom
czembower:pr/sys-auth-methods
Open

feat(sys): add auth method discovery and detail tools#87
czembower wants to merge 2 commits into
hashicorp:mainfrom
czembower:pr/sys-auth-methods

Conversation

@czembower

Copy link
Copy Markdown

Adds new tools:

  • list_auth_methods
  • read_auth_method

Registers both tools.

@czembower czembower requested a review from a team as a code owner March 5, 2026 21:39
@hashneo hashneo requested a review from Copilot March 9, 2026 22:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds new MCP tools to discover and inspect enabled Vault auth methods, and wires them into the server tool registry.

Changes:

  • Register two new sys tools in InitTools: list_auth_methods and read_auth_method.
  • Implement list_auth_methods to enumerate enabled auth mounts (optionally within a namespace).
  • Implement read_auth_method to return detailed information for a single auth mount path.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
pkg/tools/tools.go Registers the new auth-method tools with the MCP server.
pkg/tools/sys/list_auth_methods.go Adds a tool that lists enabled auth methods (with optional namespace).
pkg/tools/sys/read_auth_method.go Adds a tool that reads details for a specific auth method path (with optional namespace).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +54 to +61
// Extract parameters
args, ok := req.Params.Arguments.(map[string]interface{})
if !ok {
return mcp.NewToolResultError("Missing or invalid arguments format"), nil
}

namespace, _ := args["namespace"].(string)

Copilot AI Mar 9, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

list_auth_methods declares only an optional namespace, but the handler unconditionally type-asserts req.Params.Arguments to a map and errors if it’s nil. A CallToolRequest can omit Arguments entirely (nil), so this tool will fail when invoked with no parameters. Consider treating nil arguments as an empty map and only parsing namespace when arguments are provided (similar to other sys tools that allow optional params).

Suggested change
// Extract parameters
args, ok := req.Params.Arguments.(map[string]interface{})
if !ok {
return mcp.NewToolResultError("Missing or invalid arguments format"), nil
}
namespace, _ := args["namespace"].(string)
// Extract parameters (namespace is optional)
var namespace string
if req.Params.Arguments != nil {
args, ok := req.Params.Arguments.(map[string]interface{})
if !ok {
return mcp.NewToolResultError("Invalid arguments format"), nil
}
if ns, ok := args["namespace"].(string); ok {
namespace = ns
}
}

Copilot uses AI. Check for mistakes.
Comment on lines +128 to +140
// Include full config details
if authMethod.Config.DefaultLeaseTTL > 0 || authMethod.Config.MaxLeaseTTL > 0 {
result.Config = map[string]any{
"default_lease_ttl": authMethod.Config.DefaultLeaseTTL,
"max_lease_ttl": authMethod.Config.MaxLeaseTTL,
"force_no_cache": authMethod.Config.ForceNoCache,
"token_type": authMethod.Config.TokenType,
"audit_non_hmac_request_keys": authMethod.Config.AuditNonHMACRequestKeys,
"audit_non_hmac_response_keys": authMethod.Config.AuditNonHMACResponseKeys,
"listing_visibility": authMethod.Config.ListingVisibility,
"passthrough_request_headers": authMethod.Config.PassthroughRequestHeaders,
"allowed_response_headers": authMethod.Config.AllowedResponseHeaders,
}

Copilot AI Mar 9, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The config block is only included when DefaultLeaseTTL or MaxLeaseTTL is > 0, but other config fields (e.g., force_no_cache, headers, token_type, etc.) can be set even when TTLs are 0. This means read_auth_method can return an empty config despite the tool description claiming it returns full config details. Consider populating result.Config unconditionally (or gating on the presence of any non-zero/non-empty config field), and keep the tool description aligned with the actual output.

Suggested change
// Include full config details
if authMethod.Config.DefaultLeaseTTL > 0 || authMethod.Config.MaxLeaseTTL > 0 {
result.Config = map[string]any{
"default_lease_ttl": authMethod.Config.DefaultLeaseTTL,
"max_lease_ttl": authMethod.Config.MaxLeaseTTL,
"force_no_cache": authMethod.Config.ForceNoCache,
"token_type": authMethod.Config.TokenType,
"audit_non_hmac_request_keys": authMethod.Config.AuditNonHMACRequestKeys,
"audit_non_hmac_response_keys": authMethod.Config.AuditNonHMACResponseKeys,
"listing_visibility": authMethod.Config.ListingVisibility,
"passthrough_request_headers": authMethod.Config.PassthroughRequestHeaders,
"allowed_response_headers": authMethod.Config.AllowedResponseHeaders,
}
// Include full config details (independent of TTL values)
result.Config = map[string]any{
"default_lease_ttl": authMethod.Config.DefaultLeaseTTL,
"max_lease_ttl": authMethod.Config.MaxLeaseTTL,
"force_no_cache": authMethod.Config.ForceNoCache,
"token_type": authMethod.Config.TokenType,
"audit_non_hmac_request_keys": authMethod.Config.AuditNonHMACRequestKeys,
"audit_non_hmac_response_keys": authMethod.Config.AuditNonHMACResponseKeys,
"listing_visibility": authMethod.Config.ListingVisibility,
"passthrough_request_headers": authMethod.Config.PassthroughRequestHeaders,
"allowed_response_headers": authMethod.Config.AllowedResponseHeaders,

Copilot uses AI. Check for mistakes.
),
mcp.WithString("path",
mcp.Required(),
mcp.Description("The mount path of the auth method to read (e.g., 'approle/', 'userpass/', 'oidc/'). Include trailing slash.")),

Copilot AI Mar 9, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The path parameter description says “Include trailing slash”, but the handler normalizes the value by adding a trailing slash when missing. To avoid confusing callers, consider updating the description to indicate the trailing slash is optional (it will be added automatically).

Suggested change
mcp.Description("The mount path of the auth method to read (e.g., 'approle/', 'userpass/', 'oidc/'). Include trailing slash.")),
mcp.Description("The mount path of the auth method to read (e.g., 'approle/', 'userpass/', 'oidc/'). Trailing slash is optional; it will be added automatically if missing.")),

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants