Skip to content

Conversation

@patrickcping
Copy link
Collaborator

@patrickcping patrickcping commented Dec 18, 2025

Change Description

This PR consolidates authentication context initialization and tool invocation setup into centralized middleware, eliminating code duplication across tool handlers and simplifying updates to auth/init routines.

Key Changes:

  • Created AuthMiddleware in internal/auth/middleware/middleware.go
  • Middleware intercepts all tools/call requests and handles auth/invocation initialization
  • Removed initializeAuthContext parameter from all tool handler functions
  • Removed initialize.InitializeToolInvocation() calls from handler bodies
  • Simplified collection RegisterTools methods (removed auth-related parameters)
  • Cleaned up ~15-20 auth initialization test functions
  • Updated environment_validator.go to remove auth dependencies
  • Relocated auth test utilities to internal/testutils/auth package for better organization

Benefits:

  • Reduced code duplication significantly (removed ~200+ lines of duplicated code)
  • Centralized authentication/invocation logic in one place
  • Cleaner, more maintainable tool handlers
  • Consistent middleware pattern across all tools
  • Easier to add new tools (less boilerplate required)
  • Better test utility organization

Change Characteristics

  • This PR contains new MCP tools or resources
  • This PR requires introduction of breaking changes (internal refactoring only - no API changes)
  • No changelog entry is needed
  • This PR contains authentication/session management changes (consolidation into middleware)
  • This PR affects command-line interface

Checklist

  • Check to confirm: I have performed a review of my PR against the PR checklist and confirm that:
    • Changes have proper test coverage (including unit tests)
    • Does not introduce breaking changes (internal refactoring - no API changes)
    • Code follows the project's style guidelines and conventions
    • Documentation has been updated appropriately

Required Dependency Upgrades

N/a

Testing

This PR has been tested with:

  • Unit tests (please paste commands and results below)
  • Manual testing with MCP client (please describe testing scenario below)
  • Integration tests (please paste commands and results below)
  • Not applicable (no evidences needed)

Shell Command(s)

make test
make lint

Testing Results

Expand Test Results
go test -v -timeout 1m ./...
?       github.com/pingidentity/pingone-mcp-server      [no test files]

=== RUN   TestRootCommand
=== RUN   TestRootCommand/no_arguments
=== RUN   TestRootCommand/help_flag
=== RUN   TestRootCommand/invalid_flag
--- PASS: TestRootCommand (0.00s)
    --- PASS: TestRootCommand/no_arguments (0.00s)
    --- PASS: TestRootCommand/help_flag (0.00s)
    --- PASS: TestRootCommand/invalid_flag (0.00s)
PASS
ok      github.com/pingidentity/pingone-mcp-server/cmd  (cached)

[... output truncated for brevity ...]

=== RUN   TestEnvironmentValidationMiddleware_WithStructuredContent_OverMcp
--- PASS: TestEnvironmentValidationMiddleware_WithStructuredContent_OverMcp (0.10s)
PASS
ok      github.com/pingidentity/pingone-mcp-server/internal/tools/validation    (cached)

Test Summary:

  • ✅ All 173 unit tests pass (increased from 168 with new middleware tests)
  • ✅ Code linting passes with no issues
  • ✅ All tool handlers tested (applications, environments, populations)
  • New comprehensive middleware tests covering:
    • Authentication context initialization
    • Non-tool call pass-through
    • Invalid request type handling
    • Context propagation to tool handlers
    • Multiple sequential tool calls
    • Auto authentication flow (authorization code grant)
    • Existing session handling
    • Device code grant type support
  • ✅ Authentication flow tested through existing test suites
  • ✅ Test utilities properly organized in internal/testutils/auth package

Manual Testing Scenario

Not required - comprehensive unit test coverage validates the refactoring.

Related Issues

N/a

Moved authentication context initialization and tool invocation setup from
individual tool handlers into centralized middleware. This eliminates
significant code duplication across ~40+ tool handlers.

Key Changes:
- Created ToolInvocationMiddleware in internal/tools/initialize/middleware.go
- Middleware intercepts all tools/call requests and handles auth/invocation
- Removed initializeAuthContext parameter from all tool handler functions
- Removed initialize.InitializeToolInvocation() calls from handler bodies
- Simplified collection RegisterTools methods (removed auth-related params)
- Cleaned up ~15-20 auth initialization test functions
- Updated environment_validator.go to remove auth dependencies

Benefits:
- Reduced code duplication significantly
- Centralized authentication/invocation logic
- Cleaner, more maintainable tool handlers
- Consistent middleware pattern across all tools

Test Evidence:
All unit tests pass (168 tests):
- cmd: 6 passing
- cmd/logout: 8 passing
- cmd/run: 21 passing
- cmd/session: 9 passing
- internal/errs: 19 passing
- internal/sdk/legacy: 28 passing
- internal/server: 12 passing
- internal/tokenstore: 5 passing
- internal/tools: 2 passing
- internal/tools/applications: 21 passing
- internal/tools/environments: 44 passing
- internal/tools/filter: 9 passing
- internal/tools/populations: 20 passing
- internal/tools/types: 8 passing
- internal/tools/validation: 26 passing
- Relocate MockAuthClient and MockAuthClientFactory to testutils/auth
- Relocate MockContextInitializer functions to testutils/auth
- Add comprehensive middleware_test.go covering all auth scenarios
- Update all test imports to use authtestutils alias
- Clean up unused auth constants in collection tests
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